[Lldb-commits] [lldb] r354307 - [gui] Simplify SourceFileWindowDelegate::WindowDelegateDraw

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Feb 19 00:12:41 2019
New Revision: 354307

URL: http://llvm.org/viewvc/llvm-project?rev=354307&view=rev
Log:
[gui] Simplify SourceFileWindowDelegate::WindowDelegateDraw

instead of printf-ing into a buffer, and them using that buffer as a
format string, simply use the appropriate indirect format string.

This also fixes a -Wformat-truncation warning with gcc.

Modified:
lldb/trunk/source/Core/IOHandler.cpp

Modified: lldb/trunk/source/Core/IOHandler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/IOHandler.cpp?rev=354307&r1=354306&r2=354307&view=diff
==
--- lldb/trunk/source/Core/IOHandler.cpp (original)
+++ lldb/trunk/source/Core/IOHandler.cpp Tue Feb 19 00:12:41 2019
@@ -3929,12 +3929,10 @@ public:
 m_debugger.GetSourceManager().GetFile(m_sc.line_entry.file);
 if (m_file_sp) {
   const size_t num_lines = m_file_sp->GetNumLines();
-  int m_line_width = 1;
+  m_line_width = 1;
   for (size_t n = num_lines; n >= 10; n = n / 10)
 ++m_line_width;
 
-  snprintf(m_line_format, sizeof(m_line_format), " %%%iu ",
-   m_line_width);
   if (num_lines < num_visible_lines ||
   m_selected_line < num_visible_lines)
 m_first_visible_line = 0;
@@ -4051,7 +4049,7 @@ public:
   if (bp_attr)
 window.AttributeOn(bp_attr);
 
-  window.Printf(m_line_format, curr_line + 1);
+  window.Printf(" %*u ", m_line_width, curr_line + 1);
 
   if (bp_attr)
 window.AttributeOff(bp_attr);
@@ -4477,7 +4475,6 @@ protected:
   AddressRange m_disassembly_range;
   StreamString m_title;
   lldb::user_id_t m_tid;
-  char m_line_format[8];
   int m_line_width;
   uint32_t m_selected_line; // The selected line
   uint32_t m_pc_line;   // The line with the PC


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354308 - Fix vscode tests for python3

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Feb 19 00:25:25 2019
New Revision: 354308

URL: http://llvm.org/viewvc/llvm-project?rev=354308&view=rev
Log:
Fix vscode tests for python3

encode/decode the data before sending it over the socket. Since (AFAICT)
the vscode protocol (unlike the gdb-remote one) is fully textual, using
the utf8 codec here is appropriate.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py?rev=354308&r1=354307&r2=354308&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py Tue 
Feb 19 00:25:25 2019
@@ -54,7 +54,7 @@ def read_packet(f, verbose=False, trace_
 '''Decode a JSON packet that starts with the content length and is
followed by the JSON bytes from a file 'f'
 '''
-line = f.readline()
+line = f.readline().decode("utf-8")
 if len(line) == 0:
 return None
 
@@ -121,7 +121,7 @@ class DebugCommunication(object):
 
 @classmethod
 def encode_content(cls, s):
-return "Content-Length: %u\r\n\r\n%s" % (len(s), s)
+return ("Content-Length: %u\r\n\r\n%s" % (len(s), s)).encode("utf-8")
 
 @classmethod
 def validate_response(cls, command, response):


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42870: [lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 187350.
mgorny added a comment.

Reduced the test .yaml to the absolute minimum.


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

https://reviews.llvm.org/D42870

Files:
  lldb/lit/Modules/ELF/Inputs/netbsd-amd64.core
  lldb/lit/Modules/ELF/netbsd-core-amd64.test
  lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
  lldb/lit/Modules/lit.local.cfg
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -55,6 +55,7 @@
 const char *const LLDB_NT_OWNER_FREEBSD = "FreeBSD";
 const char *const LLDB_NT_OWNER_GNU = "GNU";
 const char *const LLDB_NT_OWNER_NETBSD = "NetBSD";
+const char *const LLDB_NT_OWNER_NETBSDCORE = "NetBSD-CORE";
 const char *const LLDB_NT_OWNER_OPENBSD = "OpenBSD";
 const char *const LLDB_NT_OWNER_CSR = "csr";
 const char *const LLDB_NT_OWNER_ANDROID = "Android";
@@ -70,8 +71,10 @@
 
 const elf_word LLDB_NT_GNU_BUILD_ID_TAG = 0x03;
 
-const elf_word LLDB_NT_NETBSD_ABI_TAG = 0x01;
-const elf_word LLDB_NT_NETBSD_ABI_SIZE = 4;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG = 1;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ = 4;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ = 7;
+const elf_word LLDB_NT_NETBSD_NT_PROCINFO = 1;
 
 // GNU ABI note OS constants
 const elf_word LLDB_NT_GNU_ABI_OS_LINUX = 0x00;
@@ -1294,25 +1297,39 @@
 // The note.n_name == LLDB_NT_OWNER_GNU is valid for Linux platform
 arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux);
 }
-// Process NetBSD ELF notes.
+// Process NetBSD ELF executables and shared libraries
 else if ((note.n_name == LLDB_NT_OWNER_NETBSD) &&
- (note.n_type == LLDB_NT_NETBSD_ABI_TAG) &&
- (note.n_descsz == LLDB_NT_NETBSD_ABI_SIZE)) {
-  // Pull out the min version info.
+ (note.n_type == LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG) &&
+ (note.n_descsz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ) &&
+ (note.n_namesz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ)) {
+  // Pull out the version info.
   uint32_t version_info;
   if (data.GetU32(&offset, &version_info, 1) == nullptr) {
 error.SetErrorString("failed to read NetBSD ABI note payload");
 return error;
   }
-
+  // Convert the version info into a major/minor/patch number.
+  // #define __NetBSD_Version__ MMmmrrpp00
+  //
+  // M = major version
+  // m = minor version; a minor number of 99 indicates current.
+  // r = 0 (since NetBSD 3.0 not used)
+  // p = patchlevel
+  const uint32_t version_major = version_info / 1;
+  const uint32_t version_minor = (version_info % 1) / 100;
+  const uint32_t version_patch = (version_info % 1) / 100;
+  // Set the elf OS version to NetBSD.  Also clear the vendor.
+  arch_spec.GetTriple().setOSName(
+  llvm::formatv("netbsd{0}.{1}.{2}", version_major, version_minor,
+version_patch).str());
+  arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
+}
+// Process NetBSD ELF core(5) notes
+else if ((note.n_name == LLDB_NT_OWNER_NETBSDCORE) &&
+ (note.n_type == LLDB_NT_NETBSD_NT_PROCINFO)) {
   // Set the elf OS version to NetBSD.  Also clear the vendor.
   arch_spec.GetTriple().setOS(llvm::Triple::OSType::NetBSD);
   arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
-
-  if (log)
-log->Printf(
-"ObjectFileELF::%s detected NetBSD, min version constant %" PRIu32,
-__FUNCTION__, version_info);
 }
 // Process OpenBSD ELF notes.
 else if (note.n_name == LLDB_NT_OWNER_OPENBSD) {
Index: lldb/lit/Modules/lit.local.cfg
===
--- lldb/lit/Modules/lit.local.cfg
+++ lldb/lit/Modules/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.s', '.yaml']
+config.suffixes = ['.s', '.test', '.yaml']
Index: lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
===
--- /dev/null
+++ lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
@@ -0,0 +1,22 @@
+# Test whether NetBSD executables are recognized correctly.
+
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Architecture: x86_64--netbsd8.99.30
+# CHECK: Type: executable
+
+--- !ELF
+FileHeader:  
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x002006F0
+Sections:
+  - Name:.note.netbsd.ident
+Type:SHT_NOTE
+Flags:   [ SHF_ALLOC ]
+Address: 

[Lldb-commits] [PATCH] D42870: [lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 187351.
mgorny added a comment.

Updated to include a non-stripped core.


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

https://reviews.llvm.org/D42870

Files:
  lldb/lit/Modules/ELF/Inputs/netbsd-amd64.core
  lldb/lit/Modules/ELF/netbsd-core-amd64.test
  lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
  lldb/lit/Modules/lit.local.cfg
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -55,6 +55,7 @@
 const char *const LLDB_NT_OWNER_FREEBSD = "FreeBSD";
 const char *const LLDB_NT_OWNER_GNU = "GNU";
 const char *const LLDB_NT_OWNER_NETBSD = "NetBSD";
+const char *const LLDB_NT_OWNER_NETBSDCORE = "NetBSD-CORE";
 const char *const LLDB_NT_OWNER_OPENBSD = "OpenBSD";
 const char *const LLDB_NT_OWNER_CSR = "csr";
 const char *const LLDB_NT_OWNER_ANDROID = "Android";
@@ -70,8 +71,10 @@
 
 const elf_word LLDB_NT_GNU_BUILD_ID_TAG = 0x03;
 
-const elf_word LLDB_NT_NETBSD_ABI_TAG = 0x01;
-const elf_word LLDB_NT_NETBSD_ABI_SIZE = 4;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG = 1;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ = 4;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ = 7;
+const elf_word LLDB_NT_NETBSD_NT_PROCINFO = 1;
 
 // GNU ABI note OS constants
 const elf_word LLDB_NT_GNU_ABI_OS_LINUX = 0x00;
@@ -1294,25 +1297,39 @@
 // The note.n_name == LLDB_NT_OWNER_GNU is valid for Linux platform
 arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux);
 }
-// Process NetBSD ELF notes.
+// Process NetBSD ELF executables and shared libraries
 else if ((note.n_name == LLDB_NT_OWNER_NETBSD) &&
- (note.n_type == LLDB_NT_NETBSD_ABI_TAG) &&
- (note.n_descsz == LLDB_NT_NETBSD_ABI_SIZE)) {
-  // Pull out the min version info.
+ (note.n_type == LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG) &&
+ (note.n_descsz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ) &&
+ (note.n_namesz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ)) {
+  // Pull out the version info.
   uint32_t version_info;
   if (data.GetU32(&offset, &version_info, 1) == nullptr) {
 error.SetErrorString("failed to read NetBSD ABI note payload");
 return error;
   }
-
+  // Convert the version info into a major/minor/patch number.
+  // #define __NetBSD_Version__ MMmmrrpp00
+  //
+  // M = major version
+  // m = minor version; a minor number of 99 indicates current.
+  // r = 0 (since NetBSD 3.0 not used)
+  // p = patchlevel
+  const uint32_t version_major = version_info / 1;
+  const uint32_t version_minor = (version_info % 1) / 100;
+  const uint32_t version_patch = (version_info % 1) / 100;
+  // Set the elf OS version to NetBSD.  Also clear the vendor.
+  arch_spec.GetTriple().setOSName(
+  llvm::formatv("netbsd{0}.{1}.{2}", version_major, version_minor,
+version_patch).str());
+  arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
+}
+// Process NetBSD ELF core(5) notes
+else if ((note.n_name == LLDB_NT_OWNER_NETBSDCORE) &&
+ (note.n_type == LLDB_NT_NETBSD_NT_PROCINFO)) {
   // Set the elf OS version to NetBSD.  Also clear the vendor.
   arch_spec.GetTriple().setOS(llvm::Triple::OSType::NetBSD);
   arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
-
-  if (log)
-log->Printf(
-"ObjectFileELF::%s detected NetBSD, min version constant %" PRIu32,
-__FUNCTION__, version_info);
 }
 // Process OpenBSD ELF notes.
 else if (note.n_name == LLDB_NT_OWNER_OPENBSD) {
Index: lldb/lit/Modules/lit.local.cfg
===
--- lldb/lit/Modules/lit.local.cfg
+++ lldb/lit/Modules/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.s', '.yaml']
+config.suffixes = ['.s', '.test', '.yaml']
Index: lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
===
--- /dev/null
+++ lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
@@ -0,0 +1,22 @@
+# Test whether NetBSD executables are recognized correctly.
+
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Architecture: x86_64--netbsd8.99.30
+# CHECK: Type: executable
+
+--- !ELF
+FileHeader:  
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x002006F0
+Sections:
+  - Name:.note.netbsd.ident
+Type:SHT_NOTE
+Flags:   [ SHF_ALLOC ]
+Address: 0x00

[Lldb-commits] [lldb] r354324 - Revert "minidump: Add ability to attach (breakpad) symbol files to placeholder modules"

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Feb 19 05:52:31 2019
New Revision: 354324

URL: http://llvm.org/viewvc/llvm-project?rev=354324&view=rev
Log:
Revert "minidump: Add ability to attach (breakpad) symbol files to placeholder 
modules"

This reverts r354263, because it uncovered a problem in handling of the
minidumps with conflicting UUIDs. If a minidump contains two files with
the same UUID, we will not create to placeholder modules for them, but
instead reuse the first one for the second instance. This creates a
problem because these modules have their load address hardcoded in them
(and I've added an assert to verify that).

Technically this is not a problem with this patch, as the same issue
existed in the previous implementation, but it did not have the assert
which would diagnose that. Nonetheless, I am reverting this until I
figure out what's the best course of action in this situation.

Removed:
lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp
lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms
lldb/trunk/lit/Minidump/breakpad-symbols.test
Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=354324&r1=354323&r2=354324&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Tue Feb 19 05:52:31 2019
@@ -163,19 +163,15 @@ public:
 lldb::ModuleSP module_sp(new Module());
 module_sp->m_objfile_sp =
 std::make_shared(module_sp, 
std::forward(args)...);
-module_sp->m_did_load_objfile.store(true, std::memory_order_relaxed);
 
-// Once we get the object file, set module ArchSpec to the one we get from
-// the object file. If the object file does not have an architecture, we
-// consider the creation a failure.
-ArchSpec arch = module_sp->m_objfile_sp->GetArchitecture();
-if (!arch)
-  return nullptr;
-module_sp->m_arch = arch;
-
-// Also copy the object file's FileSpec.
-module_sp->m_file = module_sp->m_objfile_sp->GetFileSpec();
-return module_sp;
+// Once we get the object file, update our module with the object file's
+// architecture since it might differ in vendor/os if some parts were
+// unknown.
+if (ArchSpec arch = module_sp->m_objfile_sp->GetArchitecture()) {
+  module_sp->m_arch = arch;
+  return module_sp;
+}
+return nullptr;
   }
 
   //--

Removed: lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp?rev=354323&view=auto
==
Binary files lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp (original) and 
lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp (removed) differ

Removed: lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms?rev=354323&view=auto
==
--- lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms (original)
+++ lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms (removed)
@@ -1,4 +0,0 @@
-MODULE Linux x86_64 3B285CE327C387C262DB788BF5A4078B0 linux-x86_64
-INFO CODE_ID E35C283BC327C28762DB788BF5A4078BE2351448
-FUNC 3d0 18 0 crash()
-FUNC 3f0 10 0 _start

Removed: lldb/trunk/lit/Minidump/breakpad-symbols.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/breakpad-symbols.test?rev=354323&view=auto
==
--- lldb/trunk/lit/Minidump/breakpad-symbols.test (original)
+++ lldb/trunk/lit/Minidump/breakpad-symbols.test (removed)
@@ -1,26 +0,0 @@
-# Test that we can attach breakpad symbols to the "placeholder" modules created
-# for minidump files.
-#
-# The minidump input for this test taken from 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new
-
-# RUN: %lldb -c %S/Inputs/linux-x86_64.dmp \
-# RUN:   -o "target symbols add -s /tmp/test/linux-x86_64 
%S/Inputs/linux-x86_64.syms" \
-# RUN:   -s %s -o exit | FileCheck %s
-
-image list
-# CHECK-LABEL: image list
-# CHECK: E35C283B-C327-C287-62DB-788BF5A4078B-E2351448 0x0040 
/tmp/test/linux-x86_64
-
-image dump symtab /tmp/test/linux-x86_64
-# CHECK-LABEL: image dump symtab /tmp/test/linux-x86_64
-# CHECK: Symtab, file = /tmp/test/linux-x86_64, num_symbols = 2:
-# CHECK: [0]  0   X Code0x004003d0 
0x004003d0 0x0018 0x crash()
-# CHECK: [1]  0   X Code0x004003f0 
0x004003f0 0x0010 0x _start
-
-image lookup -a 0x4003f3
-# CHECK-LABEL: 

[Lldb-commits] [PATCH] D42870: [lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 187358.
mgorny added a comment.

Switched to use shorter identifiers.


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

https://reviews.llvm.org/D42870

Files:
  lldb/lit/Modules/ELF/Inputs/netbsd-amd64.core
  lldb/lit/Modules/ELF/netbsd-core-amd64.test
  lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
  lldb/lit/Modules/lit.local.cfg
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -55,6 +55,7 @@
 const char *const LLDB_NT_OWNER_FREEBSD = "FreeBSD";
 const char *const LLDB_NT_OWNER_GNU = "GNU";
 const char *const LLDB_NT_OWNER_NETBSD = "NetBSD";
+const char *const LLDB_NT_OWNER_NETBSDCORE = "NetBSD-CORE";
 const char *const LLDB_NT_OWNER_OPENBSD = "OpenBSD";
 const char *const LLDB_NT_OWNER_CSR = "csr";
 const char *const LLDB_NT_OWNER_ANDROID = "Android";
@@ -70,8 +71,10 @@
 
 const elf_word LLDB_NT_GNU_BUILD_ID_TAG = 0x03;
 
-const elf_word LLDB_NT_NETBSD_ABI_TAG = 0x01;
-const elf_word LLDB_NT_NETBSD_ABI_SIZE = 4;
+const elf_word LLDB_NT_NETBSD_IDENT_TAG = 1;
+const elf_word LLDB_NT_NETBSD_IDENT_DESCSZ = 4;
+const elf_word LLDB_NT_NETBSD_IDENT_NAMESZ = 7;
+const elf_word LLDB_NT_NETBSD_PROCINFO = 1;
 
 // GNU ABI note OS constants
 const elf_word LLDB_NT_GNU_ABI_OS_LINUX = 0x00;
@@ -1294,25 +1297,39 @@
 // The note.n_name == LLDB_NT_OWNER_GNU is valid for Linux platform
 arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux);
 }
-// Process NetBSD ELF notes.
+// Process NetBSD ELF executables and shared libraries
 else if ((note.n_name == LLDB_NT_OWNER_NETBSD) &&
- (note.n_type == LLDB_NT_NETBSD_ABI_TAG) &&
- (note.n_descsz == LLDB_NT_NETBSD_ABI_SIZE)) {
-  // Pull out the min version info.
+ (note.n_type == LLDB_NT_NETBSD_IDENT_TAG) &&
+ (note.n_descsz == LLDB_NT_NETBSD_IDENT_DESCSZ) &&
+ (note.n_namesz == LLDB_NT_NETBSD_IDENT_NAMESZ)) {
+  // Pull out the version info.
   uint32_t version_info;
   if (data.GetU32(&offset, &version_info, 1) == nullptr) {
 error.SetErrorString("failed to read NetBSD ABI note payload");
 return error;
   }
-
+  // Convert the version info into a major/minor/patch number.
+  // #define __NetBSD_Version__ MMmmrrpp00
+  //
+  // M = major version
+  // m = minor version; a minor number of 99 indicates current.
+  // r = 0 (since NetBSD 3.0 not used)
+  // p = patchlevel
+  const uint32_t version_major = version_info / 1;
+  const uint32_t version_minor = (version_info % 1) / 100;
+  const uint32_t version_patch = (version_info % 1) / 100;
+  // Set the elf OS version to NetBSD.  Also clear the vendor.
+  arch_spec.GetTriple().setOSName(
+  llvm::formatv("netbsd{0}.{1}.{2}", version_major, version_minor,
+version_patch).str());
+  arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
+}
+// Process NetBSD ELF core(5) notes
+else if ((note.n_name == LLDB_NT_OWNER_NETBSDCORE) &&
+ (note.n_type == LLDB_NT_NETBSD_PROCINFO)) {
   // Set the elf OS version to NetBSD.  Also clear the vendor.
   arch_spec.GetTriple().setOS(llvm::Triple::OSType::NetBSD);
   arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
-
-  if (log)
-log->Printf(
-"ObjectFileELF::%s detected NetBSD, min version constant %" PRIu32,
-__FUNCTION__, version_info);
 }
 // Process OpenBSD ELF notes.
 else if (note.n_name == LLDB_NT_OWNER_OPENBSD) {
Index: lldb/lit/Modules/lit.local.cfg
===
--- lldb/lit/Modules/lit.local.cfg
+++ lldb/lit/Modules/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.s', '.yaml']
+config.suffixes = ['.s', '.test', '.yaml']
Index: lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
===
--- /dev/null
+++ lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml
@@ -0,0 +1,22 @@
+# Test whether NetBSD executables are recognized correctly.
+
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Architecture: x86_64--netbsd8.99.30
+# CHECK: Type: executable
+
+--- !ELF
+FileHeader:  
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x002006F0
+Sections:
+  - Name:.note.netbsd.ident
+Type:SHT_NOTE
+Flags:   [ SHF_ALLOC ]
+Address: 0x002005A8
+AddressAlign:0x0004
+Content:

[Lldb-commits] [PATCH] D42870: [lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks good to me. Thank you for writing the tests. @davide, do you have 
anything to add here?


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

https://reviews.llvm.org/D42870



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58347: Reinitialize UnwindTable when the SymbolFile changes

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Core/Module.cpp:1451-1454
+// Clear the unwind table too, as that may also be affected by the
+// symbol file information.
+m_unwind_table.reset();
+

Are we sure no one is holding onto info that was given out from an existing 
UnwindTable? What if we do a backtrace, then load symbol files and then do 
another backtrace? Seems like we will want to keep our existing info and just 
call m_unwind_table->Clear()?


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

https://reviews.llvm.org/D58347



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Good initiative to fix potential nullptr assignments to `std::string`.

In D55653#1400513 , @tatyana-krasnukha 
wrote:

> a caller still should examine pointers he passes to CMIUtilString::Format as 
> the ellipsis parameter.


So `Format()` and `Printf()` don't handle `nullptr` arguments already?

In D55653#1398660 , @JDevlieghere 
wrote:

> Have you considered wrapping raw strings in llvm's `StringRef`s?


+1 in general (but it may be a bigger effort)




Comment at: unittests/tools/lldb-mi/CMakeLists.txt:1
+include_directories(${LLDB_PROJECT_ROOT}/tools/lldb-mi)
+add_subdirectory(utils)

`LLDB_PROJECT_ROOT` is rarely used, maybe prefer `LLDB_SOURCE_DIR`?



Comment at: unittests/tools/lldb-mi/utils/CMakeLists.txt:3
+  ${LLDB_PROJECT_ROOT}/tools/lldb-mi/MIUtilString.cpp
+  )
+

Nice workaround for not having a library target at hand for linking!



Comment at: unittests/tools/lldb-mi/utils/StringTest.cpp:1
+//===-- LLGSTest.cpp *- C++ 
-*-===//
+//

update name


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

https://reviews.llvm.org/D55653



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58394: Add --auto-continue to stop-hooks, fix up a few tests

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: labath, jgorbe.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Auto-continuing a stop hook by adding a "continue" command to the hook didn't 
work very well for multi-threaded programs.  The first hook in kept all the 
others from running, so the behavior was unpredictable.

Also,  the stop hooks were running in synchronous mode, which meant that 
'continue' in a stop hook would start nesting hook execution, which they were 
not designed to do.

This patch forces Async execution for the stop-hooks, and adds an auto-continue 
flag.

I also made it possible to add more than one command at a time to the stop hook 
by repeating the -o command.  I can't remember why I thought it was a good idea 
originally to have only ONE --one-liner option here & in breakpoint commands, 
etc.  It's really convenient to be able to build these on a single line.

Also fixed up a few tests.  The plain stop-hook tests were failing for me, even 
though I didn't change any relevant output.  Adding the 
'interpreter.echo-comment-commands false' made them start passing again...

I didn't uncomment the UNSUPPORTED on Linux for stop-hook-threads.test,  I'd 
appreciate somebody giving this a whirl there first to see if this works now.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58394

Files:
  include/lldb/Target/Process.h
  include/lldb/Target/Target.h
  lit/ExecControl/StopHook/Inputs/stop-hook-1.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
  lit/ExecControl/StopHook/stop-hook-threads.test
  source/Commands/CommandObjectTarget.cpp
  source/Target/Process.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2554,12 +2554,14 @@
 
   StopHookCollection::iterator pos, end = m_stop_hooks.end();
 
-  // If there aren't any active stop hooks, don't bother either:
+  // If there aren't any active stop hooks, don't bother either.
+  // Also see if any of the active hooks want to auto-continue.
   bool any_active_hooks = false;
-  for (pos = m_stop_hooks.begin(); pos != end; pos++) {
-if ((*pos).second->IsActive()) {
+  bool auto_continue = false;
+  for (auto hook : m_stop_hooks) {
+if (hook.second->IsActive()) {
   any_active_hooks = true;
-  break;
+  auto_continue |= hook.second->GetAutoContinue();
 }
   }
   if (!any_active_hooks)
@@ -2595,6 +2597,7 @@
   bool hooks_ran = false;
   bool print_hook_header = (m_stop_hooks.size() != 1);
   bool print_thread_header = (num_exe_ctx != 1);
+  bool did_restart = false;
 
   for (pos = m_stop_hooks.begin(); keep_going && pos != end; pos++) {
 // result.Clear();
@@ -2639,10 +2642,13 @@
 options.SetPrintResults(true);
 options.SetAddToHistory(false);
 
+// Force Async:
+bool old_async = GetDebugger().GetAsyncExecution();
+GetDebugger().SetAsyncExecution(true);
 GetDebugger().GetCommandInterpreter().HandleCommands(
 cur_hook_sp->GetCommands(), &exc_ctx_with_reasons[i], options,
 result);
-
+GetDebugger().SetAsyncExecution(old_async);
 // If the command started the target going again, we should bag out of
 // running the stop hooks.
 if ((result.GetStatus() == eReturnStatusSuccessContinuingNoResult) ||
@@ -2651,13 +2657,19 @@
   StopHookCollection::iterator tmp = pos;
   if (++tmp != end)
 result.AppendMessageWithFormat("\nAborting stop hooks, hook %" PRIu64
-   " set the program running.\n",
+   " set the program running.\n"
+   "  Consider using '-G true' to make "
+   "stop hooks auto-continue.\n",
cur_hook_sp->GetID());
   keep_going = false;
+  did_restart = true;
 }
   }
 }
   }
+  // Finally, if auto-continue was requested, do it now:
+  if (!did_restart && auto_continue)
+m_process_sp->PrivateResume();
 
   result.GetImmediateOutputStream()->Flush();
   result.GetImmediateErrorStream()->Flush();
@@ -3143,12 +3155,13 @@
 //--
 Target::StopHook::StopHook(lldb::TargetSP target_sp, lldb::user_id_t uid)
 : UserID(uid), m_target_sp(target_sp), m_commands(), m_specifier_sp(),
-  m_thread_spec_up(), m_active(true) {}
+  m_thread_spec_up() {}
 
 Target::StopHook::StopHook(const StopHook &rhs)
 : UserID(rhs.GetID()), m_target_sp(rhs.m_target_sp),
   m_comm

[Lldb-commits] [PATCH] D58398: Add Facebook Minidump directory streams and options to dump them.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, abdulras, zturner.

Facebook creates minidump files that contain specific information about why 
things crash. Adding ways to dump these allows tools to be made that can auto 
download symbols based on the information that is contained in the minidump 
files.


https://reviews.llvm.org/D58398

Files:
  lit/Minidump/Inputs/fb-dump-content.dmp
  lit/Minidump/fb-dump.test
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpTypes.h
  source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -436,10 +436,23 @@
   OptionGroupBoolean m_dump_linux_proc_uptime;
   OptionGroupBoolean m_dump_linux_proc_fd;
   OptionGroupBoolean m_dump_linux_all;
+  OptionGroupBoolean m_fb_app_data;
+  OptionGroupBoolean m_fb_build_id;
+  OptionGroupBoolean m_fb_version;
+  OptionGroupBoolean m_fb_java_stack;
+  OptionGroupBoolean m_fb_dalvik;
+  OptionGroupBoolean m_fb_unwind;
+  OptionGroupBoolean m_fb_error_log;
+  OptionGroupBoolean m_fb_app_state;
+  OptionGroupBoolean m_fb_abort;
+  OptionGroupBoolean m_fb_thread;
+  OptionGroupBoolean m_fb_logcat;
+  OptionGroupBoolean m_fb_all;
 
   void SetDefaultOptionsIfNoneAreSet() {
 if (m_dump_all.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_all.GetOptionValue().GetCurrentValue() ||
+m_fb_all.GetOptionValue().GetCurrentValue() ||
 m_dump_directory.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_cpuinfo.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_proc_status.GetOptionValue().GetCurrentValue() ||
@@ -450,7 +463,18 @@
 m_dump_linux_maps.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_proc_stat.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_proc_uptime.GetOptionValue().GetCurrentValue() ||
-m_dump_linux_proc_fd.GetOptionValue().GetCurrentValue())
+m_dump_linux_proc_fd.GetOptionValue().GetCurrentValue() ||
+m_fb_app_data.GetOptionValue().GetCurrentValue() ||
+m_fb_build_id.GetOptionValue().GetCurrentValue() ||
+m_fb_version.GetOptionValue().GetCurrentValue() ||
+m_fb_java_stack.GetOptionValue().GetCurrentValue() ||
+m_fb_dalvik.GetOptionValue().GetCurrentValue() ||
+m_fb_unwind.GetOptionValue().GetCurrentValue() ||
+m_fb_error_log.GetOptionValue().GetCurrentValue() ||
+m_fb_app_state.GetOptionValue().GetCurrentValue() ||
+m_fb_abort.GetOptionValue().GetCurrentValue() ||
+m_fb_thread.GetOptionValue().GetCurrentValue() ||
+m_fb_logcat.GetOptionValue().GetCurrentValue())
   return;
 // If no options were set, then dump everything
 m_dump_all.GetOptionValue().SetCurrentValue(true);
@@ -505,6 +529,42 @@
 return DumpLinux() ||
 m_dump_linux_proc_fd.GetOptionValue().GetCurrentValue();
   }
+  bool DumpFacebook() const {
+return DumpAll() || m_fb_all.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookAppData() const {
+return DumpFacebook() || m_fb_app_data.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookBuildID() const {
+return DumpFacebook() || m_fb_build_id.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookVersionName() const {
+return DumpFacebook() || m_fb_version.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookJavaStack() const {
+return DumpFacebook() || m_fb_java_stack.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookDalvikInfo() const {
+return DumpFacebook() || m_fb_dalvik.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookUnwindSymbols() const {
+return DumpFacebook() || m_fb_unwind.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookErrorLog() const {
+return DumpFacebook() || m_fb_error_log.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookAppStateLog() const {
+return DumpFacebook() || m_fb_app_state.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookAbortReason() const {
+return DumpFacebook() || m_fb_abort.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookThreadName() const {
+return DumpFacebook() || m_fb_thread.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookLogcat() const {
+return DumpFacebook() || m_fb_logcat.GetOptionValue().GetCurrentValue();
+  }
 public:
 
   CommandObjectProcessMinidumpDump(CommandInterpreter &interpreter)
@@ -536,7 +596,30 @@
 INIT_BOOL(m_dump_linux_proc_fd, "fd", 'f',
   "Dump linux /proc//fd."),
 INIT_BOOL(m_dump_linux_all, "linux", 'l',
-  "Dump all linux streams.") {
+  "Dump all linux streams."),
+INIT_BOOL(m_fb_app_data, "fb-app-data", 1,
+  "Dump Facebook applicat

[Lldb-commits] [PATCH] D58398: Add Facebook Minidump directory streams and options to dump them.

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

Would be nice to change the filecheck prefixes to `CHECK-...` to separate the 
CHECK and the item being checked.


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

https://reviews.llvm.org/D58398



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

[Jonas Devlieghere via lldb-commits]

I think we'll need to reconsider this in light of the SBReproducers. The
Initialize method causes a bootstrapping problem, where we try to record
the Initialize call before the reproducer is initialized. Not recording the
initialization call isn't sufficient to solve this and in addition would
cause incorrectness in the logic that tracks API boundaries. This isn't
limited to the Initialized method, but also applies to
the SBInitializerOptions class.

I see a few potential solutions:

(1) Return a default-initialized reproducer when we try to obtain the
reproducer from the record macros before initialization. This would mean
we'd have to allow the reproducer to be re-initialized, potentially guarded
by a variable so that limits this to the aforementioned
(default-constructed) instance. This will create an imbalance, where
nothing before the initialization is recorded, but things after are. I'm
not sure if this actually causes a problem, but it feels hacky to say the
least.

(2) Introduce a static variable that says that we're currently in the
initialization phase. When the variable is set we make the record macro a
no-op. This would mean adding more code to the macro expansion. In order to
guarantee symmetry during the initialization process we'd have to set and
unset it before and after the initialize call respectively. These two new
function calls would not be instrumented and guarantee to not call any
other SB method.

(3) Go back to initializing the reproducer before the rest of the debugger.
The method wouldn't be instrumented and guarantee no other SB methods are
called or SB objects are constructed. The initialization then becomes part
of the replay. Although it's more work and basically means undoing the
changes in this patch, I think it's the cleanest from a design point of
view.

Of course I'm open to alternatives as well!

Cheers,
Jonas

On Mon, Dec 3, 2018 at 9:31 AM Jonas Devlieghere via Phabricator via
llvm-commits  wrote:

> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL348152: [Reproducers] Change how reproducers are
> initialized. (authored by JDevlieghere, committed by ).
> Herald added a subscriber: llvm-commits.
>
> Changed prior to commit:
>   https://reviews.llvm.org/D55038?vs=176161&id=176418#toc
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55038/new/
>
> https://reviews.llvm.org/D55038
>
> Files:
>   lldb/trunk/include/lldb/API/SBDebugger.h
>   lldb/trunk/include/lldb/API/SBDefines.h
>   lldb/trunk/include/lldb/API/SBFileSpec.h
>   lldb/trunk/include/lldb/API/SBInitializerOptions.h
>   lldb/trunk/include/lldb/Core/Debugger.h
>   lldb/trunk/include/lldb/Host/HostInfoBase.h
>   lldb/trunk/include/lldb/Initialization/SystemInitializer.h
>   lldb/trunk/include/lldb/Initialization/SystemInitializerCommon.h
>   lldb/trunk/include/lldb/Initialization/SystemLifetimeManager.h
>   lldb/trunk/include/lldb/Utility/Reproducer.h
>   lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in
>   lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in
>   lldb/trunk/lit/Reproducer/Inputs/simple.c
>   lldb/trunk/lit/Reproducer/TestDriverOptions.test
>   lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/reproducer/gdb-remote/Makefile
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/reproducer/gdb-remote/TestGdbRemoteReproducer.py
>   lldb/trunk/scripts/interface/SBDebugger.i
>   lldb/trunk/scripts/interface/SBInitializerOptions.i
>   lldb/trunk/scripts/lldb.swig
>   lldb/trunk/source/API/CMakeLists.txt
>   lldb/trunk/source/API/SBDebugger.cpp
>   lldb/trunk/source/API/SBInitializerOptions.cpp
>   lldb/trunk/source/API/SystemInitializerFull.cpp
>   lldb/trunk/source/API/SystemInitializerFull.h
>   lldb/trunk/source/Commands/CommandObjectReproducer.cpp
>   lldb/trunk/source/Core/Debugger.cpp
>   lldb/trunk/source/Host/common/HostInfoBase.cpp
>   lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
>   lldb/trunk/source/Initialization/SystemLifetimeManager.cpp
>   lldb/trunk/source/Utility/Reproducer.cpp
>   lldb/trunk/tools/driver/Driver.cpp
>   lldb/trunk/tools/driver/Options.td
>   lldb/trunk/tools/lldb-server/SystemInitializerLLGS.cpp
>   lldb/trunk/tools/lldb-server/SystemInitializerLLGS.h
>   lldb/trunk/tools/lldb-server/lldb-server.cpp
>   lldb/trunk/tools/lldb-test/SystemInitializerTest.cpp
>   lldb/trunk/tools/lldb-test/SystemInitializerTest.h
>   lldb/trunk/tools/lldb-test/lldb-test.cpp
>   lldb/trunk/unittests/Utility/ReproducerTest.cpp
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58347: Reinitialize UnwindTable when the SymbolFile changes

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda marked an inline comment as done.
jasonmolenda added inline comments.



Comment at: source/Core/Module.cpp:1451-1454
+// Clear the unwind table too, as that may also be affected by the
+// symbol file information.
+m_unwind_table.reset();
+

clayborg wrote:
> Are we sure no one is holding onto info that was given out from an existing 
> UnwindTable? What if we do a backtrace, then load symbol files and then do 
> another backtrace? Seems like we will want to keep our existing info and just 
> call m_unwind_table->Clear()?
I THINK it will be OK.  RegisterContextLLDB holds shared pointers to the 
UnwindPlans that it got from the UnwindTable, so those will stay alive.  An 
UnwindPlan doesn't refer back to the UnwindTable it came from.


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

https://reviews.llvm.org/D58347



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54670: 03/03: .debug_types: Update of D32167 (.debug_types) on top of D51578 (section concatenation)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am definitely ready for this to go in ASAP if we are good on this!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D54670



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: clayborg, zturner, JDevlieghere, compnerd, aprantl, 
labath.
Herald added a subscriber: jdoerfert.

While debugging an android process remotely from a windows machine, I
noticed that the modules constructed from an object file in memory only had
information about the architecture. Without knowledge of the OS or environment,
expression evaluation sometimes leads to incorrectly generated code or a
debugger crash. While we cannot know for certain what triple a module
constructed from an in-memory object file will have, a good guess is the triple
that the target executable was compiled for.


https://reviews.llvm.org/D58405

Files:
  source/Core/Module.cpp
  source/Target/Process.cpp


Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -2635,7 +2635,8 @@
 log->Printf("Process::ReadModuleFromMemory reading %s binary from memory",
 file_spec.GetPath().c_str());
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(
+  file_spec, GetTarget().GetExecutableModule()->GetArchitecture()));
   if (module_sp) {
 Status error;
 ObjectFile *objfile = module_sp->GetMemoryObjectFile(
Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -308,7 +308,7 @@
   // Once we get the object file, update our module with the object
   // file's architecture since it might differ in vendor/os if some
   // parts were unknown.
-  m_arch = m_objfile_sp->GetArchitecture();
+  m_arch.MergeFrom(m_objfile_sp->GetArchitecture());
 } else {
   error.SetErrorString("unable to find suitable object file plug-in");
 }


Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -2635,7 +2635,8 @@
 log->Printf("Process::ReadModuleFromMemory reading %s binary from memory",
 file_spec.GetPath().c_str());
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(
+  file_spec, GetTarget().GetExecutableModule()->GetArchitecture()));
   if (module_sp) {
 Status error;
 ObjectFile *objfile = module_sp->GetMemoryObjectFile(
Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -308,7 +308,7 @@
   // Once we get the object file, update our module with the object
   // file's architecture since it might differ in vendor/os if some
   // parts were unknown.
-  m_arch = m_objfile_sp->GetArchitecture();
+  m_arch.MergeFrom(m_objfile_sp->GetArchitecture());
 } else {
   error.SetErrorString("unable to find suitable object file plug-in");
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354385 - Add Facebook Minidump directory streams and options to dump them.

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Tue Feb 19 13:48:34 2019
New Revision: 354385

URL: http://llvm.org/viewvc/llvm-project?rev=354385&view=rev
Log:
Add Facebook Minidump directory streams and options to dump them.

Facebook creates minidump files that contain specific information about why 
things crash. Adding ways to dump these allows tools to be made that can auto 
download symbols based on the information that is contained in the minidump 
files.

Differential Revision: https://reviews.llvm.org/D58398


Added:
lldb/trunk/lit/Minidump/Inputs/fb-dump-content.dmp   (with props)
lldb/trunk/lit/Minidump/fb-dump.test
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Added: lldb/trunk/lit/Minidump/Inputs/fb-dump-content.dmp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Inputs/fb-dump-content.dmp?rev=354385&view=auto
==
Binary file - no diff available.

Propchange: lldb/trunk/lit/Minidump/Inputs/fb-dump-content.dmp
--
svn:mime-type = application/octet-stream

Added: lldb/trunk/lit/Minidump/fb-dump.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/fb-dump.test?rev=354385&view=auto
==
--- lldb/trunk/lit/Minidump/fb-dump.test (added)
+++ lldb/trunk/lit/Minidump/fb-dump.test Tue Feb 19 13:48:34 2019
@@ -0,0 +1,105 @@
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp \
+# RUN:   -o 'process plugin dump --all' | \
+# RUN:   FileCheck --check-prefix=CHECK-DIR --check-prefix=CHECK-APPDATA \
+# RUN:   --check-prefix=CHECK-BUILD --check-prefix=CHECK-VERSION \
+# RUN:   --check-prefix=CHECK-JAVA --check-prefix=CHECK-DALVIK \
+# RUN:   --check-prefix=CHECK-UNWIND --check-prefix=CHECK-ERROR \
+# RUN:   --check-prefix=CHECK-APPSTATE --check-prefix=CHECK-ABORT \
+# RUN:   --check-prefix=CHECK-THREAD --check-prefix=CHECK-LOGCAT %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp \
+# RUN:   -o 'process plugin dump -a' | \
+# RUN:   FileCheck --check-prefix=CHECK-DIR --check-prefix=CHECK-APPDATA \
+# RUN:   --check-prefix=CHECK-BUILD --check-prefix=CHECK-VERSION \
+# RUN:   --check-prefix=CHECK-JAVA --check-prefix=CHECK-DALVIK \
+# RUN:   --check-prefix=CHECK-UNWIND --check-prefix=CHECK-ERROR \
+# RUN:   --check-prefix=CHECK-APPSTATE --check-prefix=CHECK-ABORT \
+# RUN:   --check-prefix=CHECK-THREAD --check-prefix=CHECK-LOGCAT %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp \
+# RUN:   -o 'process plugin dump --facebook' | \
+# RUN:   FileCheck --check-prefix=CHECK-APPDATA \
+# RUN:   --check-prefix=CHECK-BUILD --check-prefix=CHECK-VERSION \
+# RUN:   --check-prefix=CHECK-JAVA --check-prefix=CHECK-DALVIK \
+# RUN:   --check-prefix=CHECK-UNWIND --check-prefix=CHECK-ERROR \
+# RUN:   --check-prefix=CHECK-APPSTATE --check-prefix=CHECK-ABORT \
+# RUN:   --check-prefix=CHECK-THREAD --check-prefix=CHECK-LOGCAT %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp \
+# RUN:   -o 'process plugin dump --fb-app-data' | \
+# RUN:   FileCheck --check-prefix=CHECK-APPDATA %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-build-id' | \
+# RUN:   FileCheck --check-prefix=CHECK-BUILD %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-version' | \
+# RUN:   FileCheck --check-prefix=CHECK-VERSION %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-java-stack' | \
+# RUN:   FileCheck --check-prefix=CHECK-JAVA %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-dalvik-info' | \
+# RUN:   FileCheck --check-prefix=CHECK-DALVIK %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-unwind-symbols' | \
+# RUN:   FileCheck --check-prefix=CHECK-UNWIND %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-error-log' | \
+# RUN:   FileCheck --check-prefix=CHECK-ERROR %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-app-state-log' | \
+# RUN:   FileCheck --check-prefix=CHECK-APPSTATE %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-abort-reason' | \
+# RUN:   FileCheck --check-prefix=CHECK-ABORT %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-thread-name' | \
+# RUN:   FileCheck --check-prefix=CHECK-THREAD %s
+# RUN: %lldb -c %p/Inputs/fb-dump-content.dmp  \
+# RUN:   -o 'process plugin dump --fb-logcat' | \
+# RUN:   FileCheck --check-prefix=CHECK-LOGCAT %s
+# CHECK-DIR:  RVASIZE   TYPE   MinidumpStreamType
+# CHECK-DIR-NEXT: -- -- -- 

[Lldb-commits] [PATCH] D58398: Add Facebook Minidump directory streams and options to dump them.

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354385: Add Facebook Minidump directory streams and options 
to dump them. (authored by gclayton, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58398?vs=187419&id=187439#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58398

Files:
  lldb/trunk/lit/Minidump/Inputs/fb-dump-content.dmp
  lldb/trunk/lit/Minidump/fb-dump.test
  lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
  lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -710,6 +710,17 @@
 ENUM_TO_CSTR(LinuxProcStat);
 ENUM_TO_CSTR(LinuxProcUptime);
 ENUM_TO_CSTR(LinuxProcFD);
+ENUM_TO_CSTR(FacebookAppCustomData);
+ENUM_TO_CSTR(FacebookBuildID);
+ENUM_TO_CSTR(FacebookAppVersionName);
+ENUM_TO_CSTR(FacebookJavaStack);
+ENUM_TO_CSTR(FacebookDalvikInfo);
+ENUM_TO_CSTR(FacebookUnwindSymbols);
+ENUM_TO_CSTR(FacebookDumpErrorLog);
+ENUM_TO_CSTR(FacebookAppStateLog);
+ENUM_TO_CSTR(FacebookAbortReason);
+ENUM_TO_CSTR(FacebookThreadName);
+ENUM_TO_CSTR(FacebookLogcat);
   }
   return "unknown stream type";
 }
Index: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -436,10 +436,23 @@
   OptionGroupBoolean m_dump_linux_proc_uptime;
   OptionGroupBoolean m_dump_linux_proc_fd;
   OptionGroupBoolean m_dump_linux_all;
+  OptionGroupBoolean m_fb_app_data;
+  OptionGroupBoolean m_fb_build_id;
+  OptionGroupBoolean m_fb_version;
+  OptionGroupBoolean m_fb_java_stack;
+  OptionGroupBoolean m_fb_dalvik;
+  OptionGroupBoolean m_fb_unwind;
+  OptionGroupBoolean m_fb_error_log;
+  OptionGroupBoolean m_fb_app_state;
+  OptionGroupBoolean m_fb_abort;
+  OptionGroupBoolean m_fb_thread;
+  OptionGroupBoolean m_fb_logcat;
+  OptionGroupBoolean m_fb_all;
 
   void SetDefaultOptionsIfNoneAreSet() {
 if (m_dump_all.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_all.GetOptionValue().GetCurrentValue() ||
+m_fb_all.GetOptionValue().GetCurrentValue() ||
 m_dump_directory.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_cpuinfo.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_proc_status.GetOptionValue().GetCurrentValue() ||
@@ -450,7 +463,18 @@
 m_dump_linux_maps.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_proc_stat.GetOptionValue().GetCurrentValue() ||
 m_dump_linux_proc_uptime.GetOptionValue().GetCurrentValue() ||
-m_dump_linux_proc_fd.GetOptionValue().GetCurrentValue())
+m_dump_linux_proc_fd.GetOptionValue().GetCurrentValue() ||
+m_fb_app_data.GetOptionValue().GetCurrentValue() ||
+m_fb_build_id.GetOptionValue().GetCurrentValue() ||
+m_fb_version.GetOptionValue().GetCurrentValue() ||
+m_fb_java_stack.GetOptionValue().GetCurrentValue() ||
+m_fb_dalvik.GetOptionValue().GetCurrentValue() ||
+m_fb_unwind.GetOptionValue().GetCurrentValue() ||
+m_fb_error_log.GetOptionValue().GetCurrentValue() ||
+m_fb_app_state.GetOptionValue().GetCurrentValue() ||
+m_fb_abort.GetOptionValue().GetCurrentValue() ||
+m_fb_thread.GetOptionValue().GetCurrentValue() ||
+m_fb_logcat.GetOptionValue().GetCurrentValue())
   return;
 // If no options were set, then dump everything
 m_dump_all.GetOptionValue().SetCurrentValue(true);
@@ -505,6 +529,42 @@
 return DumpLinux() ||
 m_dump_linux_proc_fd.GetOptionValue().GetCurrentValue();
   }
+  bool DumpFacebook() const {
+return DumpAll() || m_fb_all.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookAppData() const {
+return DumpFacebook() || m_fb_app_data.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookBuildID() const {
+return DumpFacebook() || m_fb_build_id.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookVersionName() const {
+return DumpFacebook() || m_fb_version.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookJavaStack() const {
+return DumpFacebook() || m_fb_java_stack.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookDalvikInfo() const {
+return DumpFacebook() || m_fb_dalvik.GetOptionValue().GetCurrentValue();
+  }
+  bool DumpFacebookUnwindSymbols() const {
+return DumpFacebook() || m_fb

[Lldb-commits] [PATCH] D58398: Add Facebook Minidump directory streams and options to dump them.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

BTW: I did add a - character after the CHECK before I checked it in


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58398



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

>   a good guess is the triple that the target executable was compiled for.

What does this do when the executable has multiple slices, such as a Mach-O 
universal binary?


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D58405#1402908 , @aprantl wrote:

> >   a good guess is the triple that the target executable was compiled for.
>
> What does this do when the executable has multiple slices, such as a Mach-O 
> universal binary?


Hmm, I hadn't considered that case. I'll test that and see what happens. Thanks 
for pointing that out.


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Target/Process.cpp:2638-2639
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(
+  file_spec, GetTarget().GetExecutableModule()->GetArchitecture()));
   if (module_sp) {

Is the MergeFrom in the first part not enough? I am worried about the case 
where we don't have even an executable, no one has set the architecture on the 
target, or worse yet, they have set the wrong architecture on the target. We 
want to correct the architecture on the target if we didn't specify it or the 
target was wrong. I am worried if we do this here we might hose up things in 
those cases.


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

[Alex Langford via Phabricator via lldb-commits]

xiaobai marked an inline comment as done.
xiaobai added inline comments.



Comment at: source/Target/Process.cpp:2638-2639
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(
+  file_spec, GetTarget().GetExecutableModule()->GetArchitecture()));
   if (module_sp) {

clayborg wrote:
> Is the MergeFrom in the first part not enough? I am worried about the case 
> where we don't have even an executable, no one has set the architecture on 
> the target, or worse yet, they have set the wrong architecture on the target. 
> We want to correct the architecture on the target if we didn't specify it or 
> the target was wrong. I am worried if we do this here we might hose up things 
> in those cases.
MergeFrom is not enough. When debugging an android-aarch64 binary, the triple 
was just set to `aarch64---` for modules constructed from in-memory object 
files, which is not enough info to do anything meaningful. 

However, thinking about this further, MergeFrom might not even be what we want 
here. Specifically, this is *just* a guess and the information from the 
in-memory object file is likely more reliable. It would probably better not to 
merge but to overwrite when information is available.


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54670: 03/03: .debug_types: Update of D32167 (.debug_types) on top of D51578 (section concatenation)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Few style nits




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:44
 const DWARFUnit *dwarf_cu = form_value.GetCompileUnit();
 if (dwarf_cu) {
+  // Replace the compile unit with the type signature compile unit

Invert and make this an early return.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:51
+uint64_t type_sig = form_value.Unsigned();
+auto debug_info = dwarf_cu->GetSymbolFileDWARF()->DebugInfo();
+auto tu = debug_info->GetTypeUnitForSignature(type_sig);

This `auto` is not entirely obvious for me, same for the next line.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:53
+auto tu = debug_info->GetTypeUnitForSignature(type_sig);
+if (tu) {
+  cu_offset = tu->GetOffset();

How about `if (auto tu = ...)`?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:59
+if (dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
+  cu_offset = dwarf_cu->GetBaseObjOffset();
+else

How about
```
cu_offset = dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET ? 
dwarf_cu->GetBaseObjOffset() : dwarf_cu->GetOffset();
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:64
 }
 die_offset = form_value.Reference();
   }

Move this up.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:262
+  auto signature_die = 
die.GetAttributeValueAsReferenceDIE(DW_AT_signature);
+  if (signature_die) {
+type_sp = ParseTypeFromDWARF(sc, signature_die, log, type_is_new_ptr);

How about `if (auto signature_die = ...)`?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.cpp:44
+  }
+  // reset the offset to where we tried to parse from if anything went wrong
+  *offset_ptr = cu_sp->m_offset;

Let's make this a sentence with a full stop at the end. 



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h:74
+protected:
+  // Type signature contained in a type unit which will be valid (non-zero)
+  // for type units only.

`///`



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:942
+
+// reset the offset to where we tried to parse from if anything went wrong
+*offset_ptr = m_offset;

Full sentence. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D54670



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51578: 02/03: Contiguous sections (.debug_info+.debug_types) for D54670==D32167 (.debug_types)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

Few comments inline.




Comment at: lldb/include/lldb/Symbol/ObjectFile.h:781
+friend std::unique_ptr ObjectFile::SectionReaderFactory(
+   const SectionPart §ion_part);
+SectionReader(const SectionPart §ion_part_);

Please clang-format the patch before landing this. 



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3400
+  SectionReaderCompressed(const SectionPart §ion_part_);
+  uint64_t getDecompressedSize() override;
+  size_t read(DataExtractor §ion_data) override;

Rather than returning `0` when there's no decompressor, why not return 
llvm::Optional to differentiate between with an actual size of zero? 



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3409
+  // 'decompressor' Initialize() requires already initialized 
'compressed_data'.
+  llvm::Expected decompressor;
+};

I don't think having an expected as a member makes sense. Maybe you want to use 
an optional? Is the goal to abstract over the decompression not being 
available? Or should the class assume the the decompressor is there (and itself 
be wrapped in an expected when that's not the case?). I'm not too familiar with 
this code but it seems a rather odd design. 



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3413
+ObjectFileELF::SectionReaderCompressed::SectionReaderCompressed(
+const SectionPart §ion_part_)
+: ObjectFile::SectionReader(section_part_),

Trailing underscore?



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3467
+  if (retval == 0)
+section_data.Clear();
+  else {

You could early return here (`return 0;`). 



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:684
+// present without .debug_info as that should not happen.
+if (!m_obj_file->IsInMemory() && debug_info_part
+&& !debug_info_part.GetSection()->Test(llvm::ELF::SHF_COMPRESSED)

Can we split this up? There's no way I remember the first two clauses by the 
time I'm reading the third. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D51578



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Target/Process.cpp:2639
+  ModuleSP module_sp(new Module(
+  file_spec, GetTarget().GetExecutableModule()->GetArchitecture()));
   if (module_sp) {

So if you have a target that is set to "aarch64-linux-android', and you have 
"aarch64---" in your object file/module, you would want to merge the missing 
bits from the target to augment the module's architecture so it grabs the 
"linux-android" bits. So I do believe MergeFrom should be used to augment the 
Module's architecture.


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58330: 01/03: new SectionPart for Section subranges (for effective .debug_types concatenation)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Symbol/ObjectFile.h:58
+private:
+  Section *const m_section;
+  // It should match: llvm::DWARFUnitIndex::Entry::SectionContribution

Should this be a SectionSP? If not, can we use a reference? 



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:551
   const SectionList *section_list = module_sp->GetSectionList();
   if (section_list) {
+Section *section = section_list->FindSectionByType(sect_type, true).get();

Inver the case and make this an early return?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:34
   if (section_list) {
-SectionSP section_sp(section_list->FindSectionByType(sect_type, true));
-if (section_sp) {
-  // See if we memory mapped the DWARF segment?
-  if (m_dwarf_data.GetByteSize()) {
-data.SetData(m_dwarf_data, section_sp->GetOffset(),
- section_sp->GetFileSize());
-return;
-  }
-
-  if (m_obj_file->ReadSectionData(section_sp.get(), data) != 0)
-return;
-
-  data.Clear();
-}
+Section *section = section_list->FindSectionByType(sect_type, true).get();
+if (section)

```
if (SectionSP section = section_list->FindSectionByType(sect_type, true)) 
  return SectionPart(section.get()); 
```
Would be a lot more readable imho.




Comment at: lldb/source/Symbol/ObjectFile.cpp:753
+: m_section(section), m_offset(0), m_length(section->GetFileSize()) {
+  assert(m_section);
+}

Shouldn't these be `lldbassert`s?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58330



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

[Alex Langford via Phabricator via lldb-commits]

xiaobai marked an inline comment as done.
xiaobai added inline comments.



Comment at: source/Target/Process.cpp:2639
+  ModuleSP module_sp(new Module(
+  file_spec, GetTarget().GetExecutableModule()->GetArchitecture()));
   if (module_sp) {

clayborg wrote:
> So if you have a target that is set to "aarch64-linux-android', and you have 
> "aarch64---" in your object file/module, you would want to merge the missing 
> bits from the target to augment the module's architecture so it grabs the 
> "linux-android" bits. So I do believe MergeFrom should be used to augment the 
> Module's architecture.
Ah, yes! Merging from the other direction is definitely what I wanted. Thanks 
for pointing that out.


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58330: 01/03: new SectionPart for Section subranges (for effective .debug_types concatenation)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So I question the need for the SectionPart class. Seems like it would be easier 
to just add extra args to the ReadSectionData calls? Comments?




Comment at: lldb/include/lldb/Symbol/ObjectFile.h:739
   // be larger than what Section::GetFileSize reports.
-  virtual size_t ReadSectionData(Section *section,
+  virtual size_t ReadSectionData(const SectionPart §ion_part,
  DataExtractor §ion_data);

Do we need a class here? Why not just add defaulted arguments?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:58-60
+namespace lldb_private {
+  class SectionPart;
+}

If we keep this class add it to lldb-forward.h and remove this


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58330



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58410: [Reproducers] Initialize reproducers before initializing the debugger.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added subscribers: abidh, mgorny.
Herald added a project: LLDB.

As per the discussion on the mailing list 
(http://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20190218/048007.html) 
and IRC.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58410

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBError.h
  lldb/include/lldb/API/SBFileSpec.h
  lldb/include/lldb/API/SBInitializerOptions.h
  lldb/include/lldb/API/SBReproducer.h
  lldb/include/lldb/Initialization/SystemInitializer.h
  lldb/include/lldb/Initialization/SystemInitializerCommon.h
  lldb/include/lldb/Initialization/SystemLifetimeManager.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/lldb.xcodeproj/project.pbxproj
  lldb/scripts/interface/SBDebugger.i
  lldb/scripts/interface/SBInitializerOptions.i
  lldb/scripts/lldb.swig
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBInitializerOptions.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/API/SystemInitializerFull.h
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/Initialization/SystemLifetimeManager.cpp
  lldb/source/Utility/Reproducer.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/SystemInitializerLLGS.cpp
  lldb/tools/lldb-server/SystemInitializerLLGS.h
  lldb/tools/lldb-server/lldb-server.cpp
  lldb/tools/lldb-test/SystemInitializerTest.cpp
  lldb/tools/lldb-test/SystemInitializerTest.h
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -963,7 +963,7 @@
 
   SystemLifetimeManager DebuggerLifetime;
   if (auto e = DebuggerLifetime.Initialize(
-  llvm::make_unique(), {}, nullptr)) {
+  llvm::make_unique(), nullptr)) {
 WithColor::error() << "initialization failed: " << toString(std::move(e))
<< '\n';
 return 1;
Index: lldb/tools/lldb-test/SystemInitializerTest.h
===
--- lldb/tools/lldb-test/SystemInitializerTest.h
+++ lldb/tools/lldb-test/SystemInitializerTest.h
@@ -25,7 +25,7 @@
   SystemInitializerTest();
   ~SystemInitializerTest() override;
 
-  llvm::Error Initialize(const InitializerOptions &options) override;
+  llvm::Error Initialize() override;
   void Terminate() override;
 };
 
Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -112,9 +112,8 @@
 
 SystemInitializerTest::~SystemInitializerTest() {}
 
-llvm::Error
-SystemInitializerTest::Initialize(const InitializerOptions &options) {
-  if (auto e = SystemInitializerCommon::Initialize(options))
+llvm::Error SystemInitializerTest::Initialize() {
+  if (auto e = SystemInitializerCommon::Initialize())
 return e;
 
   breakpad::ObjectFileBreakpad::Initialize();
Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -38,7 +38,7 @@
 
 static void initialize() {
   if (auto e = g_debugger_lifetime->Initialize(
-  llvm::make_unique(), {}, nullptr))
+  llvm::make_unique(), nullptr))
 llvm::consumeError(std::move(e));
 }
 
Index: lldb/tools/lldb-server/SystemInitializerLLGS.h
===
--- lldb/tools/lldb-server/SystemInitializerLLGS.h
+++ lldb/tools/lldb-server/SystemInitializerLLGS.h
@@ -14,8 +14,7 @@
 
 class SystemInitializerLLGS : public lldb_private::SystemInitializerCommon {
 public:
-  llvm::Error
-  Initialize(const lldb_private::InitializerOptions &options) override;
+  llvm::Error Initialize() override;
   void Terminate() override;
 };
 
Index: lldb/tools/lldb-server/SystemInitializerLLGS.cpp
===
--- lldb/tools/lldb-server/SystemInitializerLLGS.cpp
+++ lldb/tools/lldb-server/SystemInitializerLLGS.cpp
@@ -21,9 +21,8 @@
 
 using namespace lldb_private;
 
-llvm::Error
-SystemInitializerLLGS::Initialize(const InitializerOptions &options) {
-  if (auto e = SystemInitializerCommon::Initialize(options))
+llvm::Error SystemInitializerLLGS::Initialize() {
+  if (auto e = SystemInitializerCommon::Initialize())
 return e;
 
   HostObjectFile::Initialize();
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -13,6 +13,7 @@
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBHostOS.h"
 #include "lldb/API/SBLang

[Lldb-commits] [PATCH] D58410: [Reproducers] Initialize reproducers before initializing the debugger.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/API/SBDebugger.h:48
+
+  static lldb::SBError InitializeWithErrorHandling();
 

This should be safe, I don't know anyone relying on this and apparently the 
python `.i` file contained an incorrect return type (void). 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58410



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354400 - [lldb-instr] Don't print REGISTER macro when RECORD is already present

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb 19 15:13:29 2019
New Revision: 354400

URL: http://llvm.org/viewvc/llvm-project?rev=354400&view=rev
Log:
[lldb-instr] Don't print REGISTER macro when RECORD is already present

Currently we'd always print the LLDB_REGISTER macro, even if the
LLDB_RECORD macro was already present. This patches changes that to make
it easier to incrementally update the macros.

Note that it's still possible for the RECORD and REGISTER macros to get
out of sync.

Modified:
lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test
lldb/trunk/tools/lldb-instr/Instrument.cpp

Modified: lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test?rev=354400&r1=354399&r2=354400&view=diff
==
--- lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test (original)
+++ lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test Tue Feb 19 
15:13:29 2019
@@ -11,3 +11,4 @@
 # CHECK: LLDB_REGISTER_STATIC_METHOD(void, Foo, E, ());
 # CHECK: LLDB_REGISTER_STATIC_METHOD(int, Foo, F, (int));
 # CHECK-NOT: LLDB_REGISTER_STATIC_METHOD(void, Foo, G
+# CHECK-NOT: LLDB_REGISTER_METHOD_CONST(void, Foo, I, ());

Modified: lldb/trunk/tools/lldb-instr/Instrument.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-instr/Instrument.cpp?rev=354400&r1=354399&r2=354400&view=diff
==
--- lldb/trunk/tools/lldb-instr/Instrument.cpp (original)
+++ lldb/trunk/tools/lldb-instr/Instrument.cpp Tue Feb 19 15:13:29 2019
@@ -158,6 +158,15 @@ public:
 if (ShouldSkip(Decl))
   return false;
 
+// Skip CXXMethodDecls that already starts with a macro. This should make
+// it easier to rerun the tool to find missing macros.
+Stmt *Body = Decl->getBody();
+for (auto &C : Body->children()) {
+  if (C->getBeginLoc().isMacroID())
+return false;
+  break;
+}
+
 // Print 'bool' instead of '_Bool'.
 PrintingPolicy Policy(Context.getLangOpts());
 Policy.Bool = true;
@@ -209,14 +218,6 @@ public:
   Decl->isStatic(), Decl->isConst());
 }
 
-// If this CXXMethodDecl already starts with a macro we're done.
-Stmt *Body = Decl->getBody();
-for (auto &C : Body->children()) {
-  if (C->getBeginLoc().isMacroID())
-return false;
-  break;
-}
-
 // Insert the macro at the beginning of the function. We don't attempt to
 // fix the formatting and instead rely on clang-format to fix it after the
 // tool has run. This is also the reason that the macros end with two


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 187458.
xiaobai added a comment.

Updating based on feedback from clayborg


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

https://reviews.llvm.org/D58405

Files:
  source/Core/Module.cpp


Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -309,6 +309,10 @@
   // file's architecture since it might differ in vendor/os if some
   // parts were unknown.
   m_arch = m_objfile_sp->GetArchitecture();
+
+  // Augment the arch with the target's information in case
+  // we are unable to extract the os/environment from memory.
+  
m_arch.MergeFrom(process_sp->GetTarget().GetExecutableModule()->GetArchitecture());
 } else {
   error.SetErrorString("unable to find suitable object file plug-in");
 }


Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -309,6 +309,10 @@
   // file's architecture since it might differ in vendor/os if some
   // parts were unknown.
   m_arch = m_objfile_sp->GetArchitecture();
+
+  // Augment the arch with the target's information in case
+  // we are unable to extract the os/environment from memory.
+  m_arch.MergeFrom(process_sp->GetTarget().GetExecutableModule()->GetArchitecture());
 } else {
   error.SetErrorString("unable to find suitable object file plug-in");
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 187460.
xiaobai added a comment.

Minor change


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

https://reviews.llvm.org/D58405

Files:
  source/Core/Module.cpp


Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -309,6 +309,10 @@
   // file's architecture since it might differ in vendor/os if some
   // parts were unknown.
   m_arch = m_objfile_sp->GetArchitecture();
+
+  // Augment the arch with the target's information in case
+  // we are unable to extract the os/environment from memory.
+  m_arch.MergeFrom(process_sp->GetTarget().GetArchitecture());
 } else {
   error.SetErrorString("unable to find suitable object file plug-in");
 }


Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -309,6 +309,10 @@
   // file's architecture since it might differ in vendor/os if some
   // parts were unknown.
   m_arch = m_objfile_sp->GetArchitecture();
+
+  // Augment the arch with the target's information in case
+  // we are unable to extract the os/environment from memory.
+  m_arch.MergeFrom(process_sp->GetTarget().GetArchitecture());
 } else {
   error.SetErrorString("unable to find suitable object file plug-in");
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58394: Add --auto-continue to stop-hooks, fix up a few tests

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 187465.
jingham added a comment.

I wasn't using the right %lldb when running the tests.  That provides the 
'echo-comment-commands false'.  Run correctly you don't need the command in the 
test files.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58394

Files:
  include/lldb/Target/Process.h
  include/lldb/Target/Target.h
  lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
  lit/ExecControl/StopHook/stop-hook-threads.test
  source/Commands/CommandObjectTarget.cpp
  source/Target/Process.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2554,12 +2554,14 @@
 
   StopHookCollection::iterator pos, end = m_stop_hooks.end();
 
-  // If there aren't any active stop hooks, don't bother either:
+  // If there aren't any active stop hooks, don't bother either.
+  // Also see if any of the active hooks want to auto-continue.
   bool any_active_hooks = false;
-  for (pos = m_stop_hooks.begin(); pos != end; pos++) {
-if ((*pos).second->IsActive()) {
+  bool auto_continue = false;
+  for (auto hook : m_stop_hooks) {
+if (hook.second->IsActive()) {
   any_active_hooks = true;
-  break;
+  auto_continue |= hook.second->GetAutoContinue();
 }
   }
   if (!any_active_hooks)
@@ -2595,6 +2597,7 @@
   bool hooks_ran = false;
   bool print_hook_header = (m_stop_hooks.size() != 1);
   bool print_thread_header = (num_exe_ctx != 1);
+  bool did_restart = false;
 
   for (pos = m_stop_hooks.begin(); keep_going && pos != end; pos++) {
 // result.Clear();
@@ -2639,10 +2642,13 @@
 options.SetPrintResults(true);
 options.SetAddToHistory(false);
 
+// Force Async:
+bool old_async = GetDebugger().GetAsyncExecution();
+GetDebugger().SetAsyncExecution(true);
 GetDebugger().GetCommandInterpreter().HandleCommands(
 cur_hook_sp->GetCommands(), &exc_ctx_with_reasons[i], options,
 result);
-
+GetDebugger().SetAsyncExecution(old_async);
 // If the command started the target going again, we should bag out of
 // running the stop hooks.
 if ((result.GetStatus() == eReturnStatusSuccessContinuingNoResult) ||
@@ -2651,13 +2657,19 @@
   StopHookCollection::iterator tmp = pos;
   if (++tmp != end)
 result.AppendMessageWithFormat("\nAborting stop hooks, hook %" PRIu64
-   " set the program running.\n",
+   " set the program running.\n"
+   "  Consider using '-G true' to make "
+   "stop hooks auto-continue.\n",
cur_hook_sp->GetID());
   keep_going = false;
+  did_restart = true;
 }
   }
 }
   }
+  // Finally, if auto-continue was requested, do it now:
+  if (!did_restart && auto_continue)
+m_process_sp->PrivateResume();
 
   result.GetImmediateOutputStream()->Flush();
   result.GetImmediateErrorStream()->Flush();
@@ -3143,12 +3155,13 @@
 //--
 Target::StopHook::StopHook(lldb::TargetSP target_sp, lldb::user_id_t uid)
 : UserID(uid), m_target_sp(target_sp), m_commands(), m_specifier_sp(),
-  m_thread_spec_up(), m_active(true) {}
+  m_thread_spec_up() {}
 
 Target::StopHook::StopHook(const StopHook &rhs)
 : UserID(rhs.GetID()), m_target_sp(rhs.m_target_sp),
   m_commands(rhs.m_commands), m_specifier_sp(rhs.m_specifier_sp),
-  m_thread_spec_up(), m_active(rhs.m_active) {
+  m_thread_spec_up(), m_active(rhs.m_active),
+  m_auto_continue(rhs.m_auto_continue) {
   if (rhs.m_thread_spec_up)
 m_thread_spec_up.reset(new ThreadSpec(*rhs.m_thread_spec_up));
 }
@@ -3175,6 +3188,9 @@
   else
 s->Indent("State: disabled\n");
 
+  if (m_auto_continue)
+s->Indent("AutoContinue on\n");
+
   if (m_specifier_sp) {
 s->Indent();
 s->PutCString("Specifier:\n");
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1615,6 +1615,8 @@
   return error;
 }
 
+static const char *g_resume_sync_name = "lldb.Process.ResumeSynchronous.hijack";
+
 Status Process::ResumeSynchronous(Stream *stream) {
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
   LIBLLDB_LOG_PROCESS));
@@ -1628,7 +1630,7 @@
   }
 
   ListenerSP liste

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Can you think of a way to test this?


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

Probably just a replay of GDB protocol packets connected to a testing server 
would work.  But, I don't know if there is infrastructure for that yet.


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Perhaps a unit test?


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354415 - [lldbtest] Remove some accidentally commented out code.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Feb 19 16:54:10 2019
New Revision: 354415

URL: http://llvm.org/viewvc/llvm-project?rev=354415&view=rev
Log:
[lldbtest] Remove some accidentally commented out code.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=354415&r1=354414&r2=354415&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Tue Feb 19 16:54:10 
2019
@@ -438,8 +438,6 @@ def system(commands, **kwargs):
 stdout=PIPE,
 stderr=PIPE,
 shell=True,
-#encoding="utf-8",
-#universal_newlines=True,
 **kwargs)
 pid = process.pid
 this_output, this_error = process.communicate()


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354414 - [testsuite] Fix TestUnicodeString to work with Py2 and Py3.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Feb 19 16:54:07 2019
New Revision: 354414

URL: http://llvm.org/viewvc/llvm-project?rev=354414&view=rev
Log:
[testsuite] Fix TestUnicodeString to work with Py2 and Py3.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py?rev=354414&r1=354413&r2=354414&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py Tue Feb 19 16:54:07 
2019
@@ -5,6 +5,7 @@ from __future__ import absolute_import
 import os
 
 # Third-party modules
+import io
 
 # LLDB modules
 import lldb
@@ -13,7 +14,6 @@ from . import configuration
 from . import lldbutil
 from .decorators import *
 
-
 def source_type(filename):
 _, extension = os.path.splitext(filename)
 return {
@@ -45,7 +45,7 @@ class CommandParser:
 
 def parse_source_files(self, source_files):
 for source_file in source_files:
-file_handle = open(source_file)
+file_handle = io.open(source_file, encoding='utf-8')
 lines = file_handle.readlines()
 line_number = 0
 # non-NULL means we're looking through whitespace to find


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354418 - Move -fcxx-modules to MANDATORY_MODULE_BUILD_CFLAGS (NFC)

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Feb 19 17:14:05 2019
New Revision: 354418

URL: http://llvm.org/viewvc/llvm-project?rev=354418&view=rev
Log:
Move -fcxx-modules to MANDATORY_MODULE_BUILD_CFLAGS (NFC)

Modified:
lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules

Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules?rev=354418&r1=354417&r2=354418&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Tue Feb 19 
17:14:05 2019
@@ -284,12 +284,13 @@ endif
 
 MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules 
-fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
 
+ifeq "$(OS)" "Darwin"
+   MANDATORY_MODULE_BUILD_CFLAGS += -fcxx-modules
+endif
+
 ifeq "$(MAKE_GMODULES)" "YES"
CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
CXXFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
-   ifeq "$(OS)" "Darwin"
-   CXXFLAGS += -fcxx-modules
-   endif
 endif
 
 CXXFLAGS += -std=c++11 $(CFLAGS) $(ARCH_CXXFLAGS)


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354423 - [lldb-instr] Group RECORD macros

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb 19 17:49:10 2019
New Revision: 354423

URL: http://llvm.org/viewvc/llvm-project?rev=354423&view=rev
Log:
[lldb-instr] Group RECORD macros

Group LLDB_RECORD macros per input file.

Modified:
lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test
lldb/trunk/tools/lldb-instr/Instrument.cpp

Modified: lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test?rev=354423&r1=354422&r2=354423&view=diff
==
--- lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test (original)
+++ lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test Tue Feb 19 
17:49:10 2019
@@ -4,6 +4,7 @@
 
 # RUN: lldb-instr %t.dir/foo.cpp | FileCheck %s
 
+# CHECK: {
 # CHECK: LLDB_REGISTER_METHOD(void, Foo, A, ());
 # CHECK: LLDB_REGISTER_METHOD(void, Foo, B, (int));
 # CHECK: LLDB_REGISTER_METHOD(int, Foo, C, (int));
@@ -12,3 +13,4 @@
 # CHECK: LLDB_REGISTER_STATIC_METHOD(int, Foo, F, (int));
 # CHECK-NOT: LLDB_REGISTER_STATIC_METHOD(void, Foo, G
 # CHECK-NOT: LLDB_REGISTER_METHOD_CONST(void, Foo, I, ());
+# CHECK: }

Modified: lldb/trunk/tools/lldb-instr/Instrument.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-instr/Instrument.cpp?rev=354423&r1=354422&r2=354423&view=diff
==
--- lldb/trunk/tools/lldb-instr/Instrument.cpp (original)
+++ lldb/trunk/tools/lldb-instr/Instrument.cpp Tue Feb 19 17:49:10 2019
@@ -104,8 +104,7 @@ static std::string GetRegisterConstructo
StringRef Signature) {
   std::string Macro;
   llvm::raw_string_ostream OS(Macro);
-  OS << "LLDB_REGISTER_CONSTRUCTOR(" << Class << ", (" << Signature
- << "));\n\n";
+  OS << "LLDB_REGISTER_CONSTRUCTOR(" << Class << ", (" << Signature << "));\n";
   return OS.str();
 }
 
@@ -305,10 +304,18 @@ class SBAction : public ASTFrontendActio
 public:
   SBAction() = default;
 
-  void EndSourceFileAction() override { MyRewriter.overwriteChangedFiles(); }
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+llvm::outs() << "{\n";
+return true;
+  }
+
+  void EndSourceFileAction() override {
+llvm::outs() << "}\n";
+MyRewriter.overwriteChangedFiles();
+  }
 
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
- StringRef file) override {
+ StringRef File) override {
 MyRewriter.setSourceMgr(CI.getSourceManager(), CI.getLangOpts());
 return llvm::make_unique(MyRewriter, CI.getASTContext());
   }


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354424 - [Instrumentation] Make API logging unconditional

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb 19 17:49:13 2019
New Revision: 354424

URL: http://llvm.org/viewvc/llvm-project?rev=354424&view=rev
Log:
[Instrumentation] Make API logging unconditional

We should always log API calls in addition to logging whether the call
was recorded as part of the reproducer. Since we already have the macro
we might as well put that logic there.

Modified:
lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h

Modified: lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h?rev=354424&r1=354423&r2=354424&view=diff
==
--- lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h (original)
+++ lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h Tue Feb 19 
17:49:13 2019
@@ -29,6 +29,8 @@
   Register(static_cast(&Class::Method))
 
 #define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) 
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
 lldb_private::repro::Recorder sb_recorder( 
\
@@ -39,6 +41,8 @@
   }
 
 #define LLDB_RECORD_CONSTRUCTOR_NO_ARGS(Class) 
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
 lldb_private::repro::Recorder sb_recorder( 
\
@@ -48,6 +52,8 @@
   }
 
 #define LLDB_RECORD_METHOD(Result, Class, Method, Signature, ...)  
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   llvm::Optional sb_recorder;   
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
@@ -60,6 +66,8 @@
   }
 
 #define LLDB_RECORD_METHOD_CONST(Result, Class, Method, Signature, ...)
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   llvm::Optional sb_recorder;   
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
@@ -72,6 +80,8 @@
   }
 
 #define LLDB_RECORD_METHOD_NO_ARGS(Result, Class, Method)  
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   llvm::Optional sb_recorder;   
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
@@ -83,6 +93,8 @@
   }
 
 #define LLDB_RECORD_METHOD_CONST_NO_ARGS(Result, Class, Method)
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   llvm::Optional sb_recorder;   
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
@@ -95,6 +107,8 @@
   }
 
 #define LLDB_RECORD_STATIC_METHOD(Result, Class, Method, Signature, ...)   
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   llvm::Optional sb_recorder;   
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
@@ -105,6 +119,8 @@
   }
 
 #define LLDB_RECORD_STATIC_METHOD_NO_ARGS(Result, Class, Method)   
\
+  LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   
\
+   LLVM_PRETTY_FUNCTION);  
\
   llvm::Optional sb_recorder;   
\
   if (lldb_private::repro::InstrumentationData data =  
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
@@ -535,8 +551,8 @@ public:
 
 unsigned id = m_regist

[Lldb-commits] [lldb] r354425 - [TestModuleCXX] Use UNSUPPORTED instead of REQUIRES

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb 19 17:49:16 2019
New Revision: 354425

URL: http://llvm.org/viewvc/llvm-project?rev=354425&view=rev
Log:
[TestModuleCXX] Use UNSUPPORTED instead of REQUIRES

The requires value turns out to be bogus and the test gets skipped on
macOS.

Modified:
lldb/trunk/lit/Reproducer/Modules/TestModuleCXX.test

Modified: lldb/trunk/lit/Reproducer/Modules/TestModuleCXX.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/Modules/TestModuleCXX.test?rev=354425&r1=354424&r2=354425&view=diff
==
--- lldb/trunk/lit/Reproducer/Modules/TestModuleCXX.test (original)
+++ lldb/trunk/lit/Reproducer/Modules/TestModuleCXX.test Tue Feb 19 17:49:16 
2019
@@ -1,4 +1,4 @@
-# REQUIRES: nowindows
+# UNSUPPORTED: system-windows
 
 # Start fresh.
 # RUN: rm -rf %t.root


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58339: Changes for running LLDB test suite for Swift on PowerPC64LE

[Sarvesh Tamba via Phabricator via lldb-commits]

sarveshtamba added a comment.

Any updates on this one?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58339



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58410: [Reproducers] Initialize reproducers before initializing the debugger.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/source/API/SBReproducer.cpp:49
+SBError SBReproducer::InitializeCapture(const char *path) {
+  SBError error;
+  if (auto e = Reproducer::Initialize(ReproducerMode::Capture, FileSpec(path)))

The SBError is a problem. We can create it after the initialization, but then 
we’d need a bogus repro::Record to make sure the api boundary is correctly 
registered. Let me know if you can think of an alternative.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58410



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Augmenting the architecture with the target information sounds right to me. If 
we're reading a module from memory, then we already have a running target, and 
I consider that information more reliable (since it comes from os/lldb-server) 
than the object file architecture (which is just random bits in process 
memory). I might even go so far as to say that if we detect that the module and 
target ArchSpecs substantially differ (for some meaning of "substantially"), we 
should reject the attempt to create the module in the first place. Maybe we 
even do that already at some level?

Unfortunately, I think testing this is going to be hard. The infrastructure for 
testing in-memory object files is very under-developed. I think the best 
attempt would be to have a test program which creates some object files in 
memory (via llvm jit, or perhaps by loading pre-prepared binaries from disk), 
and then using the gdb jit interface to notify lldb about it. But I don't think 
we have anything similar to this currently.

In D58405#1402908 , @aprantl wrote:

> >   a good guess is the triple that the target executable was compiled for.
>
> What does this do when the executable has multiple slices, such as a Mach-O 
> universal binary?


I don't think this should be an issue since a Module object always points to a 
specific slice in the fat binary.


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

https://reviews.llvm.org/D58405



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r354424 - [Instrumentation] Make API logging unconditional

[Pavel Labath via lldb-commits]

On 20/02/2019 02:49, Jonas Devlieghere via lldb-commits wrote:

Author: jdevlieghere
Date: Tue Feb 19 17:49:13 2019
New Revision: 354424

URL: http://llvm.org/viewvc/llvm-project?rev=354424&view=rev
Log:
[Instrumentation] Make API logging unconditional

We should always log API calls in addition to logging whether the call
was recorded as part of the reproducer. Since we already have the macro
we might as well put that logic there.




Wohoo. I was planning to suggest that we should use this opportunity to 
fold the the manual (and often outdated) log calls into these macros, 
but then I forgot about it. :)

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits