Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv

2016-09-26 Thread Zachary Turner via lldb-commits 
zturner added a comment.

Ignore the changes to `FileSpec`, it seems the two CLs I was working on got 
mixed together.


https://reviews.llvm.org/D24952



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

[Lldb-commits] [PATCH] D24952: Remove Args::m_argv

2016-09-26 Thread Zachary Turner via lldb-commits 
zturner created this revision.
zturner added a reviewer: clayborg.
zturner added subscribers: lldb-commits, LLDB.

The `Args` class was trying to maintain two parallel argument vectors.  One 
that owned the memory and contained `std::strings`, and one that contained 
`const char *`'s that could be used similar to how `argv` is used.

But dealing with pointers to pointers is unsafe and should be avoided wherever 
possible.  Furthermore, there doesn't seem to be much point in maintaining a 
second copy of the array, as it only serves to complicate the parsing logic 
since the two have to be kept in sync with each other, and really, the master 
copy has everything you need, including the null terminated strings.

So I removed `m_argv` entirely.  The `Args` class will also no longer vend a 
`const char**`, and instead you can pass it a `std::vector&` and 
it will fill it with null terminated argument strings.  This can be used 
exactly like a `const char**` by writing `[0]`, but now you have to 
explicitly opt into this, and by default you just get a nice safe container 
that can be used for example in ranged based fors.  And the logic is simplified 
as well since the two arrays don't have to be kept in sync with each other.

All tests pass on Windows.

https://reviews.llvm.org/D24952

Files:
  examples/plugins/commands/fooplugin.cpp
  include/lldb/API/SBCommandInterpreter.h
  include/lldb/Host/FileSpec.h
  include/lldb/Host/OptionParser.h
  include/lldb/Interpreter/Args.h
  source/API/SBCommandInterpreter.cpp
  source/API/SBLaunchInfo.cpp
  source/API/SBProcess.cpp
  source/API/SBTarget.cpp
  source/Commands/CommandObjectBreakpoint.cpp
  source/Commands/CommandObjectLog.cpp
  source/Host/common/FileSpec.cpp
  source/Host/common/OptionParser.cpp
  source/Interpreter/Args.cpp
  source/Interpreter/CommandInterpreter.cpp
  source/Interpreter/CommandObject.cpp
  source/Interpreter/OptionValueArgs.cpp
  source/Interpreter/OptionValueArray.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  source/Target/ProcessInfo.cpp
  source/Target/ProcessLaunchInfo.cpp

Index: source/Target/ProcessLaunchInfo.cpp
===
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -339,8 +339,9 @@
 if (m_shell) {
   std::string shell_executable = m_shell.GetPath();
 
-  const char **argv = GetArguments().GetConstArgumentVector();
-  if (argv == nullptr || argv[0] == nullptr)
+  std::vector argv;
+  GetArguments().GetArgumentVector(argv);
+  if (!argv[0])
 return false;
   Args shell_arguments;
   std::string safe_arg;
Index: source/Target/ProcessInfo.cpp
===
--- source/Target/ProcessInfo.cpp
+++ source/Target/ProcessInfo.cpp
@@ -91,7 +91,7 @@
 
 void ProcessInfo::SetArguments(char const **argv,
bool first_arg_is_executable) {
-  m_arguments.SetArguments(argv);
+  m_arguments.SetArguments(Args::ArgvToArrayRef(argv));
 
   // Is the first argument the executable?
   if (first_arg_is_executable) {
Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -421,15 +421,14 @@
   }
 
   // Send the environment and the program + arguments after we connect
-  const char **envp =
-  launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-
-  if (envp) {
-const char *env_entry;
-for (int i = 0; (env_entry = envp[i]); ++i) {
-  if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0)
-break;
-}
+  std::vector argv;
+  launch_info.GetEnvironmentEntries().GetArgumentVector(argv);
+  const char **envp = [0];
+
+  const char *env_entry;
+  for (int i = 0; (env_entry = envp[i]); ++i) {
+if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0)
+  break;
   }
 
   ArchSpec arch_spec = launch_info.GetArchitecture();
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,13 +1402,12 @@
 // /bin/sh re-exec's itself as /bin/bash requiring another resume.
 // But it only does this if the COMMAND_MODE environment variable
 // is set to "legacy".
-const char **envp =
-launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-if (envp != NULL) {
-  for (int i = 0; envp[i] != NULL; i++) {
-if (strcmp(envp[i], "COMMAND_MODE=legacy") == 0)
-  return 2;
-  }
+std::vector argv;
+launch_info.GetEnvironmentEntries().GetArgumentVector(argv);
+const char **envp = [0];
+

Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available

2016-09-26 Thread walter erquinigo via lldb-commits 
wallace updated this revision to Diff 72597.
wallace added a comment.

minor nits


https://reviews.llvm.org/D24284

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -50,6 +50,10 @@
const lldb_private::FileSpec *file,
lldb::offset_t file_offset, lldb::offset_t length);
 
+  ObjectFilePECOFF(const lldb::ModuleSP _sp,
+   lldb::DataBufferSP _data_sp,
+   const lldb::ProcessSP _sp, lldb::addr_t header_addr);
+
   ~ObjectFilePECOFF() override;
 
   //--
@@ -130,6 +134,8 @@
 
   bool IsWindowsSubsystem();
 
+  lldb_private::DataExtractor ReadImageData(uint32_t offset, size_t size);
+
 protected:
   bool NeedsEndianSwap() const;
 
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -14,6 +14,7 @@
 
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataBuffer.h"
+#include "lldb/Core/DataBufferHeap.h"
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
@@ -83,7 +84,14 @@
 ObjectFile *ObjectFilePECOFF::CreateMemoryInstance(
 const lldb::ModuleSP _sp, lldb::DataBufferSP _sp,
 const lldb::ProcessSP _sp, lldb::addr_t header_addr) {
-  return NULL;
+  if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp))
+return nullptr;
+  auto objfile_ap = llvm::make_unique(
+  module_sp, data_sp, process_sp, header_addr);
+  if (objfile_ap.get() && objfile_ap->ParseHeader()) {
+return objfile_ap.release();
+  }
+  return nullptr;
 }
 
 size_t ObjectFilePECOFF::GetModuleSpecifications(
@@ -161,6 +169,18 @@
   ::memset(_coff_header_opt, 0, sizeof(m_coff_header_opt));
 }
 
+ObjectFilePECOFF::ObjectFilePECOFF(const lldb::ModuleSP _sp,
+   DataBufferSP _data_sp,
+   const lldb::ProcessSP _sp,
+   addr_t header_addr)
+: ObjectFile(module_sp, process_sp, header_addr, header_data_sp),
+  m_dos_header(), m_coff_header(), m_coff_header_opt(), m_sect_headers(),
+  m_entry_point_address() {
+  ::memset(_dos_header, 0, sizeof(m_dos_header));
+  ::memset(_coff_header, 0, sizeof(m_coff_header));
+  ::memset(_coff_header_opt, 0, sizeof(m_coff_header_opt));
+}
+
 ObjectFilePECOFF::~ObjectFilePECOFF() {}
 
 bool ObjectFilePECOFF::ParseHeader() {
@@ -396,6 +416,27 @@
   return success;
 }
 
+DataExtractor ObjectFilePECOFF::ReadImageData(uint32_t offset, size_t size) {
+  if (m_file) {
+DataBufferSP buffer_sp(m_file.ReadFileContents(offset, size));
+return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
+  }
+  ProcessSP process_sp(m_process_wp.lock());
+  DataExtractor data;
+  if (process_sp) {
+auto data_ap = llvm::make_unique(size, 0);
+Error readmem_error;
+size_t bytes_read =
+process_sp->ReadMemory(m_image_base + offset, data_ap->GetBytes(),
+   data_ap->GetByteSize(), readmem_error);
+if (bytes_read == size) {
+  DataBufferSP buffer_sp(data_ap.release());
+  data.SetData(buffer_sp, 0, buffer_sp->GetByteSize());
+}
+  }
+  return data;
+}
+
 //--
 // ParseSectionHeaders
 //--
@@ -405,12 +446,9 @@
   m_sect_headers.clear();
 
   if (nsects > 0) {
-const uint32_t addr_byte_size = GetAddressByteSize();
 const size_t section_header_byte_size = nsects * sizeof(section_header_t);
-DataBufferSP section_header_data_sp(m_file.ReadFileContents(
-section_header_data_offset, section_header_byte_size));
-DataExtractor section_header_data(section_header_data_sp, GetByteOrder(),
-  addr_byte_size);
+DataExtractor section_header_data =
+ReadImageData(section_header_data_offset, section_header_byte_size);
 
 lldb::offset_t offset = 0;
 if (section_header_data.ValidOffsetForDataOfSize(
@@ -470,68 +508,65 @@
 
   const uint32_t num_syms = m_coff_header.nsyms;
 
-  if (num_syms > 0 && m_coff_header.symoff > 0) {
+  if (m_file && num_syms > 0 && m_coff_header.symoff > 0) {
 const uint32_t symbol_size = 18;
-const uint32_t addr_byte_size = GetAddressByteSize();
 const size_t symbol_data_size = num_syms * symbol_size;
 // Include the 4-byte string table size at the end of the symbols
-DataBufferSP

Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available

2016-09-26 Thread walter erquinigo via lldb-commits 
wallace updated this revision to Diff 72596.
wallace added a comment.

This time I'm calculating the address of the exports header correctly because 
it is
an RVA address.


https://reviews.llvm.org/D24284

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -50,6 +50,10 @@
const lldb_private::FileSpec *file,
lldb::offset_t file_offset, lldb::offset_t length);
 
+  ObjectFilePECOFF(const lldb::ModuleSP _sp,
+   lldb::DataBufferSP _data_sp,
+   const lldb::ProcessSP _sp, lldb::addr_t header_addr);
+
   ~ObjectFilePECOFF() override;
 
   //--
@@ -130,6 +134,8 @@
 
   bool IsWindowsSubsystem();
 
+  lldb_private::DataExtractor ReadImageData(uint32_t offset, size_t size);
+
 protected:
   bool NeedsEndianSwap() const;
 
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -14,6 +14,7 @@
 
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataBuffer.h"
+#include "lldb/Core/DataBufferHeap.h"
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
@@ -83,6 +84,13 @@
 ObjectFile *ObjectFilePECOFF::CreateMemoryInstance(
 const lldb::ModuleSP _sp, lldb::DataBufferSP _sp,
 const lldb::ProcessSP _sp, lldb::addr_t header_addr) {
+  if (data_sp && ObjectFilePECOFF::MagicBytesMatch(data_sp)) {
+std::auto_ptr objfile_ap(
+new ObjectFilePECOFF(module_sp, data_sp, process_sp, header_addr));
+if (objfile_ap.get() && objfile_ap->ParseHeader()) {
+  return objfile_ap.release();
+}
+  }
   return NULL;
 }
 
@@ -161,6 +169,18 @@
   ::memset(_coff_header_opt, 0, sizeof(m_coff_header_opt));
 }
 
+ObjectFilePECOFF::ObjectFilePECOFF(const lldb::ModuleSP _sp,
+   DataBufferSP _data_sp,
+   const lldb::ProcessSP _sp,
+   addr_t header_addr)
+: ObjectFile(module_sp, process_sp, header_addr, header_data_sp),
+  m_dos_header(), m_coff_header(), m_coff_header_opt(), m_sect_headers(),
+  m_entry_point_address() {
+  ::memset(_dos_header, 0, sizeof(m_dos_header));
+  ::memset(_coff_header, 0, sizeof(m_coff_header));
+  ::memset(_coff_header_opt, 0, sizeof(m_coff_header_opt));
+}
+
 ObjectFilePECOFF::~ObjectFilePECOFF() {}
 
 bool ObjectFilePECOFF::ParseHeader() {
@@ -396,6 +416,27 @@
   return success;
 }
 
+DataExtractor ObjectFilePECOFF::ReadImageData(uint32_t offset, size_t size) {
+  if (m_file) {
+DataBufferSP buffer_sp(m_file.ReadFileContents(offset, size));
+return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
+  }
+  ProcessSP process_sp(m_process_wp.lock());
+  DataExtractor data;
+  if (process_sp) {
+std::unique_ptr data_ap(new DataBufferHeap(size, 0));
+Error readmem_error;
+size_t bytes_read =
+process_sp->ReadMemory(m_image_base + offset, data_ap->GetBytes(),
+   data_ap->GetByteSize(), readmem_error);
+if (bytes_read == size) {
+  DataBufferSP buffer_sp(data_ap.release());
+  data.SetData(buffer_sp, 0, buffer_sp->GetByteSize());
+}
+  }
+  return data;
+}
+
 //--
 // ParseSectionHeaders
 //--
@@ -407,10 +448,8 @@
   if (nsects > 0) {
 const uint32_t addr_byte_size = GetAddressByteSize();
 const size_t section_header_byte_size = nsects * sizeof(section_header_t);
-DataBufferSP section_header_data_sp(m_file.ReadFileContents(
-section_header_data_offset, section_header_byte_size));
-DataExtractor section_header_data(section_header_data_sp, GetByteOrder(),
-  addr_byte_size);
+DataExtractor section_header_data =
+ReadImageData(section_header_data_offset, section_header_byte_size);
 
 lldb::offset_t offset = 0;
 if (section_header_data.ValidOffsetForDataOfSize(
@@ -470,68 +509,65 @@
 
   const uint32_t num_syms = m_coff_header.nsyms;
 
-  if (num_syms > 0 && m_coff_header.symoff > 0) {
+  if (m_file && num_syms > 0 && m_coff_header.symoff > 0) {
 const uint32_t symbol_size = 18;
-const uint32_t addr_byte_size = GetAddressByteSize();
 const size_t symbol_data_size = num_syms * symbol_size;
 // Include the 4-byte string table size at the end of the symbols
-

Re: [Lldb-commits] [PATCH] D13154: [MIPS] Use Address::GetAddressClass() instead of elf flags to decide address space of an address

2016-09-26 Thread Eugene Zelenko via lldb-commits 
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Looks like patch was not committed.


Repository:
  rL LLVM

https://reviews.llvm.org/D13154



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

Re: [Lldb-commits] [PATCH] D13350: [lldb] Fix evaluation of qualified global variables

2016-09-26 Thread Eugene Zelenko via lldb-commits 
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Looks like patch was not committed.


https://reviews.llvm.org/D13350



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

Re: [Lldb-commits] [PATCH] D13578: Allow generic arm ArchSpec to merge with specific arm ArchSpec; allow Cortex M0-7's to always force thumb mode

2016-09-26 Thread Eugene Zelenko via lldb-commits 
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in https://reviews.llvm.org/rL265377.


Repository:
  rL LLVM

https://reviews.llvm.org/D13578



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

Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible

2016-09-26 Thread Muhammad Omair Javaid via lldb-commits 
omjavaid updated this revision to Diff 72589.
omjavaid added a comment.

This is a new version of what seems to me fully implementing functionality we 
intend to have here.

On a second thought nuking ClearHardwareWatchpoint function seems to be the 
wrong approach here. I spent some time taking different approaches and it turns 
out that if we do not ClearHardwareWatchpoint when back-end asks us to remove 
it then we wont be able to step over watchpoints. On ARM targets we have to 
first clear and then reinstall watchpoints to step over the watchpoint 
instruction.

On the other hand if we call 
NativeRegisterContextLinux_arm::ClearHardwareWatchpoint then that watchpoint 
stands removed if call is just to delete watch on one of the bytes. And if we 
follow up with creating a new watchpoint on a different word the slot being 
used may appear vaccant which is actually inconsistent behavior.

So I have a new approach that does clear watchpoint registers if 
NativeRegisterContextLinux_arm::ClearHardwareWatchpoint is called but we still 
track reference counts by re-introducing refcount that I removed in my last 
patch. This will mean that a follow up create may fail just because there are 
still references to disabled watchpoint and watchpoint slots are still not 
vaccant. I have made changes to the test to reflect this behaviour.

Please comment if you have any reservation about this approach.


https://reviews.llvm.org/D24610

Files:
  
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
  
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
  
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
  source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
  source/Plugins/Process/Linux/NativeThreadLinux.cpp

Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -198,8 +198,10 @@
   m_stop_info.reason = StopReason::eStopReasonNone;
   m_stop_description.clear();
 
-  // If watchpoints have been set, but none on this thread,
-  // then this is a new thread. So set all existing watchpoints.
+  // Invalidate watchpoint index map for a re-sync
+  m_watchpoint_index_map.clear();
+
+  // Re-sync all available watchpoints.
   if (m_watchpoint_index_map.empty()) {
 NativeProcessLinux  = GetProcess();
 
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -508,8 +508,19 @@
   if (error.Fail())
 return LLDB_INVALID_INDEX32;
 
-  uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0;
+  uint32_t control_value = 0;
   lldb::addr_t real_addr = addr;
+  uint32_t wp_index = LLDB_INVALID_INDEX32;
+
+  // Find out how many bytes we need to watch after 4-byte alignment boundary.
+  uint8_t watch_size = (addr & 0x03) + size;
+
+  // We cannot watch zero or more than 4 bytes after 4-byte alignment boundary.
+  if (size == 0 || watch_size > 4)
+return LLDB_INVALID_INDEX32;
+
+  // Strip away last two bits of address for byte/half-word/word selection.
+  addr &= ~((lldb::addr_t)3);
 
   // Check if we are setting watchpoint other than read/write/access
   // Also update watchpoint flag to match Arm write-read bit configuration.
@@ -526,86 +537,63 @@
 return LLDB_INVALID_INDEX32;
   }
 
-  // Can't watch zero bytes
-  // Can't watch more than 4 bytes per WVR/WCR pair
+  // Iterate over stored watchpoints and find a free or duplicate wp_index
+  for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+if ((m_hwp_regs[i].control & 1) == 0 && (m_hwp_regs[i].refcount <= 0)) {
+  wp_index = i; // Mark last free slot
+} else if (m_hwp_regs[i].address == addr) {
+  wp_index = i; // Mark duplicate index
+  break;// Stop searching here
+}
+  }
 
-  if (size == 0 || size > 4)
+  // No vaccant slot available and no duplicate slot found.
+  if (wp_index == LLDB_INVALID_INDEX32)
 return LLDB_INVALID_INDEX32;
 
-  // Check 4-byte alignment for hardware watchpoint target address.
-  // Below is a hack to recalculate address and size in order to
-  // make sure we can watch non 4-byte alligned addresses as well.
-  if (addr & 0x03) {
-uint8_t watch_mask = (addr & 0x03) + size;
-
-if (watch_mask > 0x04)
-  return LLDB_INVALID_INDEX32;
-else if (watch_mask <= 0x02)
-  size = 2;
-else if (watch_mask <= 0x04)
-  size = 4;
-
-addr = addr & (~0x03);
+  uint8_t current_watch_size, new_watch_size;
+  // Calculate overall size width to be watched by current hardware watchpoint slot.
+  current_watch_size =

Re: [Lldb-commits] [PATCH] D14136: Refactor Windows process plugin to enable sharing of code between live and post-mortem debugging

2016-09-26 Thread Eugene Zelenko via lldb-commits 
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in https://reviews.llvm.org/rL251540.


https://reviews.llvm.org/D14136



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

Re: [Lldb-commits] [lldb] r282445 - Fix an issue where LLDB would not accept the --description-verbosity option to 'po' without an argument after the StringRef refactoring

2016-09-26 Thread Enrico Granata via lldb-commits 

> On Sep 26, 2016, at 3:03 PM, Zachary Turner  wrote:
> 
> Oh cool. Is it not hooked up to the public waterfall?

That's a really good question...
It's marked @skipUnlessDarwin, but that's about all that is remarkable about 
it; I know about the failure because someone internally at Apple filed a bug 
report about it, but didn't get any bot emails complaining (and apparently 
neither did you)

Weird.. With that said, if you care to try, it's expression_command/po_verbosity

> On Mon, Sep 26, 2016 at 3:02 PM Enrico Granata  > wrote:
>> On Sep 26, 2016, at 2:50 PM, Zachary Turner > > wrote:
>> 
>> Test would be nice, but otoh command line tests are discouraged. Maybe it 
>> will be easier to write this kind of test if we had a tool specifically for 
>> testing command line options similar to what I proposed with unwinding etc. 
>> oh well
> 
> There already is a test for this - that's how the breakage was caught to 
> begin with :-)
> 
>> On Mon, Sep 26, 2016 at 2:45 PM Enrico Granata via lldb-commits 
>> > wrote:
>> Author: enrico
>> Date: Mon Sep 26 16:36:17 2016
>> New Revision: 282445
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=282445=rev 
>> 
>> Log:
>> Fix an issue where LLDB would not accept the --description-verbosity option 
>> to 'po' without an argument after the StringRef refactoring
>> 
>> Fixes rdar://28480275 <>
>> 
>> 
>> Modified:
>> lldb/trunk/source/Commands/CommandObjectExpression.cpp
>> 
>> Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=282445=282444=282445=diff
>>  
>> 
>> ==
>> --- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
>> +++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Mon Sep 26 
>> 16:36:17 2016
>> @@ -142,7 +142,7 @@ Error CommandObjectExpression::CommandOp
>>}
>> 
>>case 'v':
>> -if (!option_arg.empty()) {
>> +if (option_arg.empty()) {
>>m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityFull;
>>break;
>>  }
>> 
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org 
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
>> 
> 
> 
> 
> Thanks,
> - Enrico
>  egranata@.com ☎️ 27683
> 


Thanks,
- Enrico
 egranata@.com ☎️ 27683

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

Re: [Lldb-commits] [lldb] r282445 - Fix an issue where LLDB would not accept the --description-verbosity option to 'po' without an argument after the StringRef refactoring

2016-09-26 Thread Zachary Turner via lldb-commits 
Oh cool. Is it not hooked up to the public waterfall?
On Mon, Sep 26, 2016 at 3:02 PM Enrico Granata  wrote:

> On Sep 26, 2016, at 2:50 PM, Zachary Turner  wrote:
>
> Test would be nice, but otoh command line tests are discouraged. Maybe it
> will be easier to write this kind of test if we had a tool specifically for
> testing command line options similar to what I proposed with unwinding etc.
> oh well
>
>
> There already is a test for this - that's how the breakage was caught to
> begin with :-)
>
> On Mon, Sep 26, 2016 at 2:45 PM Enrico Granata via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
> Author: enrico
> Date: Mon Sep 26 16:36:17 2016
> New Revision: 282445
>
> URL: http://llvm.org/viewvc/llvm-project?rev=282445=rev
> Log:
> Fix an issue where LLDB would not accept the --description-verbosity
> option to 'po' without an argument after the StringRef refactoring
>
> Fixes rdar://28480275
>
>
> Modified:
> lldb/trunk/source/Commands/CommandObjectExpression.cpp
>
> Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=282445=282444=282445=diff
>
> ==
> --- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
> +++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Mon Sep 26
> 16:36:17 2016
> @@ -142,7 +142,7 @@ Error CommandObjectExpression::CommandOp
>}
>
>case 'v':
> -if (!option_arg.empty()) {
> +if (option_arg.empty()) {
>m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityFull;
>break;
>  }
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
>
> Thanks,
> *- Enrico*
>  egranata@.com ☎️ 27683
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282445 - Fix an issue where LLDB would not accept the --description-verbosity option to 'po' without an argument after the StringRef refactoring

2016-09-26 Thread Enrico Granata via lldb-commits 

> On Sep 26, 2016, at 2:50 PM, Zachary Turner  wrote:
> 
> Test would be nice, but otoh command line tests are discouraged. Maybe it 
> will be easier to write this kind of test if we had a tool specifically for 
> testing command line options similar to what I proposed with unwinding etc. 
> oh well

There already is a test for this - that's how the breakage was caught to begin 
with :-)

> On Mon, Sep 26, 2016 at 2:45 PM Enrico Granata via lldb-commits 
> > wrote:
> Author: enrico
> Date: Mon Sep 26 16:36:17 2016
> New Revision: 282445
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=282445=rev 
> 
> Log:
> Fix an issue where LLDB would not accept the --description-verbosity option 
> to 'po' without an argument after the StringRef refactoring
> 
> Fixes rdar://28480275
> 
> 
> Modified:
> lldb/trunk/source/Commands/CommandObjectExpression.cpp
> 
> Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=282445=282444=282445=diff
>  
> 
> ==
> --- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
> +++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Mon Sep 26 
> 16:36:17 2016
> @@ -142,7 +142,7 @@ Error CommandObjectExpression::CommandOp
>}
> 
>case 'v':
> -if (!option_arg.empty()) {
> +if (option_arg.empty()) {
>m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityFull;
>break;
>  }
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org 
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> 


Thanks,
- Enrico
 egranata@.com ☎️ 27683

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

Re: [Lldb-commits] [lldb] r282445 - Fix an issue where LLDB would not accept the --description-verbosity option to 'po' without an argument after the StringRef refactoring

2016-09-26 Thread Zachary Turner via lldb-commits 
Test would be nice, but otoh command line tests are discouraged. Maybe it
will be easier to write this kind of test if we had a tool specifically for
testing command line options similar to what I proposed with unwinding etc.
oh well
On Mon, Sep 26, 2016 at 2:45 PM Enrico Granata via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: enrico
> Date: Mon Sep 26 16:36:17 2016
> New Revision: 282445
>
> URL: http://llvm.org/viewvc/llvm-project?rev=282445=rev
> Log:
> Fix an issue where LLDB would not accept the --description-verbosity
> option to 'po' without an argument after the StringRef refactoring
>
> Fixes rdar://28480275
>
>
> Modified:
> lldb/trunk/source/Commands/CommandObjectExpression.cpp
>
> Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=282445=282444=282445=diff
>
> ==
> --- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
> +++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Mon Sep 26
> 16:36:17 2016
> @@ -142,7 +142,7 @@ Error CommandObjectExpression::CommandOp
>}
>
>case 'v':
> -if (!option_arg.empty()) {
> +if (option_arg.empty()) {
>m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityFull;
>break;
>  }
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r282445 - Fix an issue where LLDB would not accept the --description-verbosity option to 'po' without an argument after the StringRef refactoring

2016-09-26 Thread Enrico Granata via lldb-commits 
Author: enrico
Date: Mon Sep 26 16:36:17 2016
New Revision: 282445

URL: http://llvm.org/viewvc/llvm-project?rev=282445=rev
Log:
Fix an issue where LLDB would not accept the --description-verbosity option to 
'po' without an argument after the StringRef refactoring

Fixes rdar://28480275


Modified:
lldb/trunk/source/Commands/CommandObjectExpression.cpp

Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=282445=282444=282445=diff
==
--- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Mon Sep 26 16:36:17 
2016
@@ -142,7 +142,7 @@ Error CommandObjectExpression::CommandOp
   }
 
   case 'v':
-if (!option_arg.empty()) {
+if (option_arg.empty()) {
   m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityFull;
   break;
 }


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

Re: [Lldb-commits] [PATCH] D24936: Make FileSpec use StringRef.

2016-09-26 Thread Greg Clayton via lldb-commits 
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Watch the buildbots for failures, but this looks fine. We aren't changing how 
the strings for filename and directory are stored, just using StringRef to 
deliver the arguments.


https://reviews.llvm.org/D24936



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

Re: [Lldb-commits] [PATCH] D24890: implement timeout sample support for Linux

2016-09-26 Thread Todd Fiala via lldb-commits 
tfiala added a comment.

BTW, regarding this part:

> On Ubuntu 16.04, the requisite support can be retrieved with:

> 

> sudo apt-get install perf-tools-unstable


This was incorrect.  The perf tool was actually present even without the 
perf-tools-unstable on Ubuntu 16.04 x86_64.  It is just that there are more 
tools for processing perf output available with that package, but this change 
doesn't require any of them.  /usr/bin/perf comes from the linux-tools-common 
package on Ubuntu.  I tried testing with and without that package installed.


Repository:
  rL LLVM

https://reviews.llvm.org/D24890



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

[Lldb-commits] [PATCH] D24936: Make FileSpec use StringRef.

2016-09-26 Thread Zachary Turner via lldb-commits 
zturner created this revision.
zturner added a reviewer: clayborg.
zturner added a subscriber: lldb-commits.
zturner added a dependency: D24880: Add StringExtras join_items function.

This patch depends on D24880 going in for the `join_items` function, but at 
least you can comment on it now.



https://reviews.llvm.org/D24936

Files:
  include/lldb/Host/FileSpec.h
  source/Host/common/FileSpec.cpp

Index: source/Host/common/FileSpec.cpp
===
--- source/Host/common/FileSpec.cpp
+++ source/Host/common/FileSpec.cpp
@@ -37,6 +37,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/CleanUp.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/FileSystem.h"
@@ -228,27 +229,27 @@
 #endif
 }
 
-size_t FileSpec::ResolvePartialUsername(const char *partial_name,
+size_t FileSpec::ResolvePartialUsername(llvm::StringRef partial_name,
 StringList ) {
 #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER
   size_t extant_entries = matches.GetSize();
 
   setpwent();
   struct passwd *user_entry;
-  const char *name_start = partial_name + 1;
+  partial_name = partial_name.drop_front();
   std::set name_list;
 
   while ((user_entry = getpwent()) != NULL) {
-if (strstr(user_entry->pw_name, name_start) == user_entry->pw_name) {
+if (llvm::StringRef(user_entry->pw_name).startswith(partial_name)) {
   std::string tmp_buf("~");
   tmp_buf.append(user_entry->pw_name);
   tmp_buf.push_back('/');
   name_list.insert(tmp_buf);
 }
   }
-  std::set::iterator pos, end = name_list.end();
-  for (pos = name_list.begin(); pos != end; pos++) {
-matches.AppendString((*pos).c_str());
+
+  for (auto  : name_list) {
+matches.AppendString(name);
   }
   return matches.GetSize() - extant_entries;
 #else
@@ -284,23 +285,15 @@
 // Default constructor that can take an optional full path to a
 // file on disk.
 //--
-FileSpec::FileSpec(const char *pathname, bool resolve_path, PathSyntax syntax)
+FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, PathSyntax syntax)
 : m_directory(), m_filename(), m_is_resolved(false), m_syntax(syntax) {
-  if (pathname && pathname[0])
-SetFile(pathname, resolve_path, syntax);
+  SetFile(path, resolve_path, syntax);
 }
 
-FileSpec::FileSpec(const char *pathname, bool resolve_path, ArchSpec arch)
-: FileSpec{pathname, resolve_path, arch.GetTriple().isOSWindows()
-   ? ePathSyntaxWindows
-   : ePathSyntaxPosix} {}
-
-FileSpec::FileSpec(const std::string , bool resolve_path,
-   PathSyntax syntax)
-: FileSpec{path.c_str(), resolve_path, syntax} {}
-
-FileSpec::FileSpec(const std::string , bool resolve_path, ArchSpec arch)
-: FileSpec{path.c_str(), resolve_path, arch} {}
+FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch)
+: FileSpec{path, resolve_path, arch.GetTriple().isOSWindows()
+   ? ePathSyntaxWindows
+   : ePathSyntaxPosix} {}
 
 //--
 // Copy constructor
@@ -340,7 +333,12 @@
 // be split up into a directory and filename and stored as uniqued
 // string values for quick comparison and efficient memory usage.
 //--
-void FileSpec::SetFile(const char *pathname, bool resolve, PathSyntax syntax) {
+void FileSpec::SetFile(llvm::StringRef pathname, bool resolve,
+   PathSyntax syntax) {
+  // CLEANUP: Use StringRef for string handling.  This function is kind of a
+  // mess and the unclear semantics of RootDirStart and ParentPathEnd make
+  // it very difficult to understand this function.  There's no reason this
+  // function should be particularly complicated or difficult to understand.
   m_filename.Clear();
   m_directory.Clear();
   m_is_resolved = false;
@@ -348,7 +346,7 @@
  ? FileSystem::GetNativePathSyntax()
  : syntax;
 
-  if (pathname == NULL || pathname[0] == '\0')
+  if (pathname.empty())
 return;
 
   llvm::SmallString<64> resolved(pathname);
@@ -382,20 +380,10 @@
: resolve_path_ref.substr(filename_begin));
 }
 
-void FileSpec::SetFile(const char *pathname, bool resolve, ArchSpec arch) {
-  return SetFile(pathname, resolve, arch.GetTriple().isOSWindows()
-? ePathSyntaxWindows
-: ePathSyntaxPosix);
-}
-
-void FileSpec::SetFile(const std::string , bool resolve,
-   PathSyntax syntax) {
-  return SetFile(pathname.c_str(), resolve, syntax);
-}
-
-void FileSpec::SetFile(const std::string , bool resolve,
-

Re: [Lldb-commits] [PATCH] D24890: implement timeout sample support for Linux

2016-09-26 Thread Todd Fiala via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL282436: added Linux support for test timeout sampling 
(authored by tfiala).

Changed prior to commit:
  https://reviews.llvm.org/D24890?vs=72382=72553#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24890

Files:
  lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py
  lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
  lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_linux.py
  lldb/trunk/packages/Python/lldbsuite/test/dosep.py
  lldb/trunk/packages/Python/lldbsuite/test/test_runner/process_control.py

Index: lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py
===
--- lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py
+++ lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py
@@ -0,0 +1,76 @@
+"""Provides a pre-kill method to run on Linux.
+
+This timeout pre-kill method relies on the Linux perf-tools
+distribution.  The appropriate way to obtain this set of tools
+will depend on the Linux distribution.
+
+For Ubuntu 16.04, the invoke the following command:
+sudo apt-get install perf-tools-unstable
+"""
+from __future__ import print_function
+
+# system imports
+import os
+import subprocess
+import sys
+import tempfile
+
+
+def do_pre_kill(process_id, runner_context, output_stream, sample_time=3):
+"""Samples the given process id, and puts the output to output_stream.
+
+@param process_id the local process to sample.
+
+@param runner_context a dictionary of details about the architectures
+and platform on which the given process is running.  Expected keys are
+archs (array of architectures), platform_name, platform_url, and
+platform_working_dir.
+
+@param output_stream file-like object that should be used to write the
+results of sampling.
+
+@param sample_time specifies the time in seconds that should be captured.
+"""
+
+# Validate args.
+if runner_context is None:
+raise Exception("runner_context argument is required")
+if not isinstance(runner_context, dict):
+raise Exception("runner_context argument must be a dictionary")
+
+# We will try to run sample on the local host only if there is no URL
+# to a remote.
+if "platform_url" in runner_context and (
+runner_context["platform_url"] is not None):
+import pprint
+sys.stderr.write(
+"warning: skipping timeout pre-kill sample invocation because we "
+"don't know how to run on a remote yet. runner_context={}\n"
+.format(pprint.pformat(runner_context)))
+
+# We're going to create a temp file, and immediately overwrite it with the
+# following command.  This just ensures we don't have any races in
+# creation of the temporary sample file.
+fileno, filename = tempfile.mkstemp(suffix='perfdata')
+os.close(fileno)
+fileno = None
+
+try:
+with open(os.devnull, 'w') as devnull:
+returncode = subprocess.call(['timeout', str(sample_time), 'perf',
+  'record', '-g', '-o', filename, '-p', str(process_id)],
+ stdout=devnull, stderr=devnull)
+if returncode == 0 or returncode == 124:
+# This is okay - this is the timeout return code, which is totally
+# expected.
+pass
+else:
+raise Exception("failed to call 'perf record .., error: {}".format(
+returncode))
+
+with open(os.devnull, 'w') as devnull:
+output = subprocess.check_output(['perf', 'report', '--call-graph',
+  '--stdio', '-i', filename], stderr=devnull)
+output_stream.write(output)
+finally:
+os.remove(filename)
Index: lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
===
--- lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
+++ lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
@@ -38,7 +38,7 @@
 print("parent: sending shut-down request to child")
 if self.process:
 self.child_work_queue.put("hello, child")
-self.process.join()
+self.process.join()
 if self.verbose:
 print("parent: child is fully shut down")
 
Index: lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_linux.py
===
--- lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_linux.py
+++ lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_linux.py
@@ -0,0 +1,133 @@
+"""Test the pre-kill hook on Linux."""
+from __future__ import print_function
+
+# system imports
+from multiprocessing import Process, Queue

[Lldb-commits] [lldb] r282436 - added Linux support for test timeout sampling

2016-09-26 Thread Todd Fiala via lldb-commits 
Author: tfiala
Date: Mon Sep 26 15:25:47 2016
New Revision: 282436

URL: http://llvm.org/viewvc/llvm-project?rev=282436=rev
Log:
added Linux support for test timeout sampling

This is the Linux counterpart to the sampling support I added
on the macOS side.

This change also introduces zip-file compression if the size of
the sample output is greater than 10 KB.  The Linux side can be
quite large and the textual content is averaging over a 10x
compression factor on tests that I force to time out.  When
compression takes place, the filename becomes:

{session_dir}/{TestFilename.py}-{pid}.sample.zip

This support relies on the linux 'perf' tool.  If it isn't
present, the behavior is to ignore pre-kill processing of
the timed out test process.

Note calling the perf tool under the timeout command appears
to nuke the profiled process.  This was causing the timeout
kill logic to fail due to the process having disappeared.
I modified the kill logic to catch the case of the process
not existing, and I have it ignore the kill request in that
case.  Any other exception is still raised.

Reviewers: labath

Subscribers: lldb-commits

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

Added:
lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py
lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_linux.py
  - copied, changed from r282432, 
lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
Modified:
lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
lldb/trunk/packages/Python/lldbsuite/test/dosep.py
lldb/trunk/packages/Python/lldbsuite/test/test_runner/process_control.py

Added: lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py?rev=282436=auto
==
--- lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py (added)
+++ lldb/trunk/packages/Python/lldbsuite/pre_kill_hook/linux.py Mon Sep 26 
15:25:47 2016
@@ -0,0 +1,76 @@
+"""Provides a pre-kill method to run on Linux.
+
+This timeout pre-kill method relies on the Linux perf-tools
+distribution.  The appropriate way to obtain this set of tools
+will depend on the Linux distribution.
+
+For Ubuntu 16.04, the invoke the following command:
+sudo apt-get install perf-tools-unstable
+"""
+from __future__ import print_function
+
+# system imports
+import os
+import subprocess
+import sys
+import tempfile
+
+
+def do_pre_kill(process_id, runner_context, output_stream, sample_time=3):
+"""Samples the given process id, and puts the output to output_stream.
+
+@param process_id the local process to sample.
+
+@param runner_context a dictionary of details about the architectures
+and platform on which the given process is running.  Expected keys are
+archs (array of architectures), platform_name, platform_url, and
+platform_working_dir.
+
+@param output_stream file-like object that should be used to write the
+results of sampling.
+
+@param sample_time specifies the time in seconds that should be captured.
+"""
+
+# Validate args.
+if runner_context is None:
+raise Exception("runner_context argument is required")
+if not isinstance(runner_context, dict):
+raise Exception("runner_context argument must be a dictionary")
+
+# We will try to run sample on the local host only if there is no URL
+# to a remote.
+if "platform_url" in runner_context and (
+runner_context["platform_url"] is not None):
+import pprint
+sys.stderr.write(
+"warning: skipping timeout pre-kill sample invocation because we "
+"don't know how to run on a remote yet. runner_context={}\n"
+.format(pprint.pformat(runner_context)))
+
+# We're going to create a temp file, and immediately overwrite it with the
+# following command.  This just ensures we don't have any races in
+# creation of the temporary sample file.
+fileno, filename = tempfile.mkstemp(suffix='perfdata')
+os.close(fileno)
+fileno = None
+
+try:
+with open(os.devnull, 'w') as devnull:
+returncode = subprocess.call(['timeout', str(sample_time), 'perf',
+  'record', '-g', '-o', filename, 
'-p', str(process_id)],
+ stdout=devnull, stderr=devnull)
+if returncode == 0 or returncode == 124:
+# This is okay - this is the timeout return code, which is totally
+# expected.
+pass
+else:
+raise Exception("failed to call 'perf record .., error: {}".format(
+returncode))
+
+with open(os.devnull, 'w') as devnull:
+output = subprocess.check_output(['perf', 'report', '--call-graph',
+

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-09-26 Thread Eugene Zelenko via lldb-commits 
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Looks like patch was not committed.


https://reviews.llvm.org/D17635



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

[Lldb-commits] [lldb] r282434 - Added a setting that enables saving all .o files from a given JIT expression.

2016-09-26 Thread Sean Callanan via lldb-commits 
Author: spyffe
Date: Mon Sep 26 15:18:51 2016
New Revision: 282434

URL: http://llvm.org/viewvc/llvm-project?rev=282434=rev
Log:
Added a setting that enables saving all .o files from a given JIT expression.

This allows debugging of the JIT and other analyses of the internals of the
expression parser.  I've also added a testcase that verifies that the setting
works correctly when off and on.

Added:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/TestSaveJITObjects.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/main.c
Modified:
lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/Expression/IRExecutionUnit.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/IRExecutionUnit.h?rev=282434=282433=282434=diff
==
--- lldb/trunk/include/lldb/Expression/IRExecutionUnit.h (original)
+++ lldb/trunk/include/lldb/Expression/IRExecutionUnit.h Mon Sep 26 15:18:51 
2016
@@ -33,6 +33,7 @@ namespace llvm {
 
 class Module;
 class ExecutionEngine;
+class ObjectCache;
 
 } // namespace llvm
 
@@ -398,6 +399,7 @@ private:
 
   std::unique_ptr m_context_ap;
   std::unique_ptr m_execution_engine_ap;
+  std::unique_ptr m_object_cache_ap;
   std::unique_ptr
   m_module_ap;///< Holder for the module until it's been handed off
   llvm::Module *m_module; ///< Owned by the execution engine

Modified: lldb/trunk/include/lldb/Target/Target.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=282434=282433=282434=diff
==
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Mon Sep 26 15:18:51 2016
@@ -128,6 +128,8 @@ public:
 
   bool GetEnableNotifyAboutFixIts() const;
 
+  bool GetEnableSaveObjects() const;
+
   bool GetEnableSyntheticValue() const;
 
   uint32_t GetMaximumNumberOfChildrenToDisplay() const;

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/Makefile?rev=282434=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/Makefile
 Mon Sep 26 15:18:51 2016
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/TestSaveJITObjects.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/TestSaveJITObjects.py?rev=282434=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/TestSaveJITObjects.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/save_jit_objects/TestSaveJITObjects.py
 Mon Sep 26 15:18:51 2016
@@ -0,0 +1,57 @@
+"""
+Test that LLDB can emit JIT objects when the appropriate setting is enabled
+"""
+
+from __future__ import print_function
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+def enumerateJITFiles():
+return [f for f in os.listdir(os.getcwd()) if f.startswith("jit")]
+
+def countJITFiles():
+return len(enumerateJITFiles())
+
+def cleanJITFiles():
+for j in enumerateJITFiles():
+os.remove(j)
+return
+
+class SaveJITObjectsTestCase(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def test_save_jit_objects(self):
+self.build()
+src_file = "main.c"
+src_file_spec = lldb.SBFileSpec(src_file)
+  
+exe_path = os.path.join(os.getcwd(), "a.out")
+target = self.dbg.CreateTarget(exe_path)
+
+breakpoint = target.BreakpointCreateBySourceRegex(
+"break", src_file_spec)
+
+process = target.LaunchSimple(None, None,
+  self.get_process_working_directory())
+
+thread = process.GetSelectedThread()
+frame = thread.GetSelectedFrame()
+
+cleanJITFiles()
+frame.EvaluateExpression("(void*)malloc(0x1)")
+self.assertTrue(countJITFiles() == 0,
+

[Lldb-commits] [lldb] r282432 - Fix serialization of Python breakpoint commands.

2016-09-26 Thread Jim Ingham via lldb-commits 
Author: jingham
Date: Mon Sep 26 14:47:37 2016
New Revision: 282432

URL: http://llvm.org/viewvc/llvm-project?rev=282432=rev
Log:
Fix serialization of Python breakpoint commands.

CommandData breakpoint commands didn't know whether they were
Python or Command line commands, so they couldn't serialize &
deserialize themselves properly.  Fix that.
I also changed the "breakpoint list" command to note in the output
when the commands are Python commands.  Fortunately only one test
was relying on this explicit bit of text output.

Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
lldb/trunk/include/lldb/lldb-enumerations.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
lldb/trunk/source/API/SBBreakpoint.cpp
lldb/trunk/source/Breakpoint/Breakpoint.cpp
lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/trunk/source/Interpreter/ScriptInterpreter.cpp

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h?rev=282432=282431=282432=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h Mon Sep 26 14:47:37 
2016
@@ -34,10 +34,13 @@ namespace lldb_private {
 class BreakpointOptions {
 public:
   struct CommandData {
-CommandData() : user_source(), script_source(), stop_on_error(true) {}
-
-CommandData(const StringList _source)
-: user_source(user_source), script_source(), stop_on_error(true) {}
+CommandData()
+: user_source(), script_source(),
+  interpreter(lldb::eScriptLanguageNone), stop_on_error(true) {}
+
+CommandData(const StringList _source, lldb::ScriptLanguage interp)
+: user_source(user_source), script_source(), interpreter(interp),
+  stop_on_error(true) {}
 
 ~CommandData() = default;
 
@@ -51,12 +54,14 @@ public:
 
 StringList user_source;
 std::string script_source;
+enum lldb::ScriptLanguage
+interpreter; // eScriptLanguageNone means command interpreter.
 bool stop_on_error;
 
   private:
 enum class OptionNames : uint32_t {
   UserSource = 0,
-  ScriptSource,
+  Interpreter,
   StopOnError,
   LastOptionName
 };
@@ -112,7 +117,8 @@ public:
   virtual ~BreakpointOptions();
 
   static std::unique_ptr
-  CreateFromStructuredData(const StructuredData::Dictionary _dict,
+  CreateFromStructuredData(Target ,
+   const StructuredData::Dictionary _dict,
Error );
 
   virtual StructuredData::ObjectSP SerializeToStructuredData();
@@ -366,16 +372,16 @@ protected:
 OneShotState,
 LastOptionName
   };
-  static const char *g_option_names[(size_t) OptionNames::LastOptionName];
+  static const char *g_option_names[(size_t)OptionNames::LastOptionName];
 
   static const char *GetKey(OptionNames enum_value) {
-return g_option_names[(size_t) enum_value];
+return g_option_names[(size_t)enum_value];
   }
 
   static bool BreakpointOptionsCallbackFunction(
   void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id,
   lldb::user_id_t break_loc_id);
-  
+
   void SetThreadSpec(std::unique_ptr _spec_up);
 
 private:

Modified: lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h?rev=282432=282431=282432=diff
==
--- lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h (original)
+++ lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h Mon Sep 26 14:47:37 
2016
@@ -16,6 +16,7 @@
 // Project includes
 #include "lldb/lldb-private.h"
 
+#include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Core/Broadcaster.h"
 #include "lldb/Core/Error.h"
 #include "lldb/Core/PluginInterface.h"
@@ -270,6 +271,15 @@ public:
 return error;
   }
 
+  /// This one is for deserialization:
+  virtual Error SetBreakpointCommandCallback(
+  BreakpointOptions *bp_options,
+  std::unique_ptr _up) {
+Error error;
+error.SetErrorString("unimplemented");
+return error;
+  }
+
   void SetBreakpointCommandCallbackFunction(
   std::vector _options_vec,
   const char *function_name);
@@ -428,8 +438,12 @@ public:
 
   static std::string

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Dimitar Vlahovski via lldb-commits 
dvlahovski added inline comments.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:47-48
@@ +46,4 @@
+
+  if (*context_flags & uint32_t(MinidumpContext_x86_64_Flags::Control)) {
+writeRegister(source_data, result_base, _info[lldb_cs_x86_64], 2);
+  }

amccarth wrote:
> dvlahovski wrote:
> > If it is then when I do a `&` the result is an enum class of type 
> > `MinidumpContext_x86_64_Flags`. And the compiler complains that this is not 
> > convertible to bool
> I think what Zach means is that you could locally define a uint32_t const, 
> initialized with the value from the enum.  Then each if statement could use 
> that constant without a cast.
> 
> Also, is this right?  `MinidumpContext_x86_x64_Flags::Control` has two bits 
> set, so the condition will be true if either of them is set.  Is that the 
> intended behavior?  Or should you be ensuring that they're both set like this:
> 
> const utin32_t ControlFlags = MinidumpContext_x86_64::Control;
> if ((*context_flags & ControlFlags) == ControlFlags) {
>   ...
> }
> 
> ?
Now that I think about I should check the arch flag bit at the beggining of 
this.
But then this if's are OK I think - in them I only want to check if that 
specific bit is set.


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Adrian McCarthy via lldb-commits 
amccarth added inline comments.


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:1
@@ +1,2 @@
+//===-- Registers_x86_64.cpp *- C++ 
-*-===//
+//

Should match file name.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:31
@@ +30,3 @@
+// specified in the RegisterInfoInterface argument
+// This way we can reuse the already existing register contexts
+lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContextToRegIface(

It might be better to put this comment with the declaration in the header file, 
since it explains what to pass in and what it does.  Comments in .cpp files 
should contain implementation details that the callers don't necessarily need 
to know.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:47-48
@@ +46,4 @@
+
+  if (*context_flags & uint32_t(MinidumpContext_x86_64_Flags::Control)) {
+writeRegister(source_data, result_base, _info[lldb_cs_x86_64], 2);
+  }

dvlahovski wrote:
> If it is then when I do a `&` the result is an enum class of type 
> `MinidumpContext_x86_64_Flags`. And the compiler complains that this is not 
> convertible to bool
I think what Zach means is that you could locally define a uint32_t const, 
initialized with the value from the enum.  Then each if statement could use 
that constant without a cast.

Also, is this right?  `MinidumpContext_x86_x64_Flags::Control` has two bits 
set, so the condition will be true if either of them is set.  Is that the 
intended behavior?  Or should you be ensuring that they're both set like this:

const utin32_t ControlFlags = MinidumpContext_x86_64::Control;
if ((*context_flags & ControlFlags) == ControlFlags) {
  ...
}

?


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:1
@@ +1,2 @@
+//===-- Registers_x86_64.h --*- C++ 
-*-===//
+//

Please make this line match the file name.


Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+static void registerEqualToVal(const uint64_t val, uint8_t *reg_val) {
+  ASSERT_EQ(val, *(reinterpret_cast(reg_val)));
+}

+1 to Zach's idea.

Also, consider whether `EXPECT_EQ` is more appropriate than `ASSERT_EQ` for 
these.


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Zachary Turner via lldb-commits 
zturner added inline comments.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:49
@@ +48,3 @@
+writeRegister(source_data, result_base, _info[lldb_cs_x86_64], 2);
+  }
+

dvlahovski wrote:
> sizeof(uint16_t), sizeof(uint32_t), etc ?
I think my comments got out of line and this is no longer at the position I 
suggested it.  that said, I actually didn't notice all these constants being 
passed into `writeRegister`.  I would propose changing this.  Find the 
`RegisterInfo` class in `lldb-private-types.h` and add the following methods:

```
llvm::ArrayRef data(const uint8_t *context_base) const { 
  return llvm::ArrayRef(context_base + byte_offset, byte_size);
}

llvm::MutableArrayRef mutable_data(uint8_t *context_base) const {
  return llvm::MutableArrayRef(context_base + byte_offset, byte_size);
}
```

Then you can write:

```writeRegister(source_data, 
reg_info[lldb_cs_x86_64].mutable_data(result_base));```


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Dimitar Vlahovski via lldb-commits 
dvlahovski added inline comments.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:49
@@ +48,3 @@
+writeRegister(source_data, result_base, _info[lldb_cs_x86_64], 2);
+  }
+

sizeof(uint16_t), sizeof(uint32_t), etc ?


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Dimitar Vlahovski via lldb-commits 
dvlahovski added inline comments.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:47-48
@@ +46,4 @@
+
+  if (*context_flags & uint32_t(MinidumpContext_x86_64_Flags::Control)) {
+writeRegister(source_data, result_base, _info[lldb_cs_x86_64], 2);
+  }

If it is then when I do a `&` the result is an enum class of type 
`MinidumpContext_x86_64_Flags`. And the compiler complains that this is not 
convertible to bool


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Pavel Labath via lldb-commits 
labath added a comment.

lgtm, after Zachary is happy.


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Dimitar Vlahovski via lldb-commits 
dvlahovski added a comment.

I will fix the comments that Zachary made in the next revision


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Dimitar Vlahovski via lldb-commits 
dvlahovski updated this revision to Diff 72509.
dvlahovski marked 4 inline comments as done.
dvlahovski added a comment.

Updating the CL regarding Pavel's comments


https://reviews.llvm.org/D24919

Files:
  source/Plugins/Process/minidump/CMakeLists.txt
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
  source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
  unittests/Process/minidump/MinidumpParserTest.cpp

Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -8,16 +8,19 @@
 //===--===//
 
 // Project includes
+#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
 #include "Plugins/Process/minidump/MinidumpParser.h"
 #include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
 
 // Other libraries and framework includes
 #include "gtest/gtest.h"
 
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataExtractor.h"
 #include "lldb/Host/FileSpec.h"
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -50,7 +53,7 @@
 MinidumpParser::Create(data_sp);
 ASSERT_TRUE(optional_parser.hasValue());
 parser.reset(new MinidumpParser(optional_parser.getValue()));
-ASSERT_GT(parser->GetByteSize(), 0UL);
+ASSERT_GT(parser->GetData().size(), 0UL);
   }
 
   llvm::SmallString<128> inputs_folder;
@@ -167,3 +170,63 @@
   ASSERT_TRUE(pid.hasValue());
   ASSERT_EQ(4440UL, pid.getValue());
 }
+
+// Register stuff
+// TODO probably split register stuff tests into different file?
+static void registerEqualToVal(const uint64_t val, uint8_t *reg_val) {
+  ASSERT_EQ(val, *(reinterpret_cast(reg_val)));
+}
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
+  SetUpData("linux-x86_64.dmp");
+  llvm::ArrayRef thread_list = parser->GetThreads();
+  const MinidumpThread thread = thread_list[0];
+  llvm::ArrayRef registers(parser->GetData().data() +
+thread.thread_context.rva,
+thread.thread_context.data_size);
+
+  ArchSpec arch = parser->GetArchitecture();
+  RegisterInfoInterface *reg_interface = new RegisterContextLinux_x86_64(arch);
+  lldb::DataBufferSP buf =
+  ConvertMinidumpContextToRegIface(registers, reg_interface);
+  ASSERT_EQ(reg_interface->GetGPRSize(), buf->GetByteSize());
+
+  const RegisterInfo *reg_info = reg_interface->GetRegisterInfo();
+
+  std::map reg_values;
+
+  // clang-format off
+  reg_values[lldb_rax_x86_64]=  0x;
+  reg_values[lldb_rbx_x86_64]=  0x;
+  reg_values[lldb_rcx_x86_64]=  0x0010;
+  reg_values[lldb_rdx_x86_64]=  0x;
+  reg_values[lldb_rdi_x86_64]=  0x7ffceb349cf0;
+  reg_values[lldb_rsi_x86_64]=  0x;
+  reg_values[lldb_rbp_x86_64]=  0x7ffceb34a210;
+  reg_values[lldb_rsp_x86_64]=  0x7ffceb34a210;
+  reg_values[lldb_r8_x86_64] =  0x7fe9bc1aa9c0;
+  reg_values[lldb_r9_x86_64] =  0x;
+  reg_values[lldb_r10_x86_64]=  0x7fe9bc3f16a0;
+  reg_values[lldb_r11_x86_64]=  0x0246;
+  reg_values[lldb_r12_x86_64]=  0x00401c92;
+  reg_values[lldb_r13_x86_64]=  0x7ffceb34a430;
+  reg_values[lldb_r14_x86_64]=  0x;
+  reg_values[lldb_r15_x86_64]=  0x;
+  reg_values[lldb_rip_x86_64]=  0x00401dc6;
+  reg_values[lldb_rflags_x86_64] =  0x00010206;
+  reg_values[lldb_cs_x86_64] =  0x0033;
+  reg_values[lldb_fs_x86_64] =  0x;
+  reg_values[lldb_gs_x86_64] =  0x;
+  reg_values[lldb_ss_x86_64] =  0x;
+  reg_values[lldb_ds_x86_64] =  0x;
+  reg_values[lldb_es_x86_64] =  0x;
+  // clang-format on
+
+  for (uint32_t reg_index = 0; reg_index < reg_interface->GetRegisterCount();
+   ++reg_index) {
+if (reg_values.find(reg_index) != reg_values.end()) {
+  registerEqualToVal(reg_values[reg_index],
+ buf->GetBytes() + reg_info[reg_index].byte_offset);
+}
+  }
+}
Index: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
===
--- /dev/null
+++ source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -0,0 +1,114 @@
+//===-- Registers_x86_64.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Zachary Turner via lldb-commits 
zturner added inline comments.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:46-47
@@ +45,4 @@
+  source_data = source_data.drop_front(6 * 8); // p[1-6] home registers
+  const uint32_t *context_flags;
+  consumeObject(source_data, context_flags);
+  source_data = source_data.drop_front(4); // mx_csr

Can this be a `const MinidumpContext_x86_64_Flags*`?  Then you wouldn't have to 
do the `uint32_t` conversion each time since you could just `&` the flag you're 
checking directly against this value.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:48
@@ +47,3 @@
+  consumeObject(source_data, context_flags);
+  source_data = source_data.drop_front(4); // mx_csr
+

Can we use a `sizeof()` here?


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:37
@@ +36,3 @@
+struct MinidumpXmmSaveArea32AMD64 {
+  uint16_t control_word;
+  uint16_t status_word;

Should these be endian specific types like ulittle16_t?  If someone writes a 
minidump on a little endian system and runs this code on a big endian system, 
it would fail to parse the minidump correctly.


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:161
@@ +160,3 @@
+
+  static void writeRegister(llvm::ArrayRef , uint8_t *base,
+const RegisterInfo *reg_info, size_t size);

labath wrote:
> This looks like a private utility function. It should not be in the header.
The order of parameters here is confusing.  the output buffer should be 
adjacent to its length.  So it should be:

`void writeRegister(ArrayRef<>, uint8_t*, size_t, const RegisterInfo*)`

That said, passing around pointers and lengths is a pattern that needs to die.  
I would much rather this be:

`void writeRegister(ArrayRef _src, MutableArrayRef 
reg_dest, const RegisterInfo );`


Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+void registerEqualToVal(const uint64_t val, uint8_t *reg_val) {
+  ASSERT_EQ(val, *(reinterpret_cast(reg_val)));
+}

I would make the conversion a macro like this:

`#define REG_VAL(x) *(reinterpret_cast(x))`

Then write the asserts inline.  The reason for this is that when you do it this 
way, when a test fails the test runner will report that the failure happened in 
`registerEqualToVal`, which won't give you any clue about what test it failed 
in.  if the assertion is kept in the body of the test, the failure will report 
which test it was in, so you can more easily see.


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Pavel Labath via lldb-commits 
labath added inline comments.


Comment at: source/Plugins/Process/minidump/MinidumpParser.h:42
@@ -41,1 +41,3 @@
 
+  const uint8_t *GetBaseAddr();
+

Replace these two functions with `llvm::ArrayRef GetData() const`


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:55
@@ +54,3 @@
+
+struct MinidumpContext_x86_64 {
+  // Register parameter home addresses.

This function does not reflect the actual memory layout of the data, and you 
are not using it. I think it should go away.


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:161
@@ +160,3 @@
+
+  static void writeRegister(llvm::ArrayRef , uint8_t *base,
+const RegisterInfo *reg_info, size_t size);

This looks like a private utility function. It should not be in the header.


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:186
@@ +185,3 @@
+};
+}
+}

Could you add ` // end namespace minidump` comments. Without them it's hard to 
tell what is going on here.


Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:176
@@ +175,3 @@
+// TODO probably split register stuff tests into different file?
+void registerEqualToVal(const uint64_t val, uint8_t *reg_val) {
+  ASSERT_EQ(val, *(reinterpret_cast(reg_val)));

`static`

Also i think having the register tests in this file is fine.


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Dimitar Vlahovski via lldb-commits 
dvlahovski added a comment.

I tested this on the yet-to-be-submitted plugin code and it works fine on Linux 
(should work fine everywhere for that matter). I can do a backtrace, go up/down 
the stack frames and, of course, see all of the registers with `register read`


https://reviews.llvm.org/D24919



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

[Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

2016-09-26 Thread Dimitar Vlahovski via lldb-commits 
dvlahovski created this revision.
dvlahovski added reviewers: labath, zturner.
dvlahovski added subscribers: amccarth, lldb-commits.
Herald added subscribers: mgorny, beanz.

This is a register context converter from Minidump to Linux reg context.
This knows the layout of the register context in the Minidump file
(which is the same as in Windows FYI) and as a result emits a binary data
buffer that matches the Linux register context binary layout.
This way we can reuse the existing RegisterContextLinux_x86_64 and
RegisterContextCorePOSIX_x86_64 classes.

https://reviews.llvm.org/D24919

Files:
  source/Plugins/Process/minidump/CMakeLists.txt
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
  source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
  unittests/Process/minidump/MinidumpParserTest.cpp

Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -8,16 +8,19 @@
 //===--===//
 
 // Project includes
+#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
 #include "Plugins/Process/minidump/MinidumpParser.h"
 #include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
 
 // Other libraries and framework includes
 #include "gtest/gtest.h"
 
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataExtractor.h"
 #include "lldb/Host/FileSpec.h"
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -167,3 +170,63 @@
   ASSERT_TRUE(pid.hasValue());
   ASSERT_EQ(4440UL, pid.getValue());
 }
+
+// Register stuff
+// TODO probably split register stuff tests into different file?
+void registerEqualToVal(const uint64_t val, uint8_t *reg_val) {
+  ASSERT_EQ(val, *(reinterpret_cast(reg_val)));
+}
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
+  SetUpData("linux-x86_64.dmp");
+  llvm::ArrayRef thread_list = parser->GetThreads();
+  const MinidumpThread thread = thread_list[0];
+  llvm::ArrayRef registers(parser->GetBaseAddr() +
+thread.thread_context.rva,
+thread.thread_context.data_size);
+
+  ArchSpec arch = parser->GetArchitecture();
+  RegisterInfoInterface *reg_interface = new RegisterContextLinux_x86_64(arch);
+  lldb::DataBufferSP buf =
+  MinidumpContext_x86_64::Convert(registers, reg_interface);
+  ASSERT_EQ(reg_interface->GetGPRSize(), buf->GetByteSize());
+
+  const RegisterInfo *reg_info = reg_interface->GetRegisterInfo();
+
+  std::map reg_values;
+
+  // clang-format off
+  reg_values[lldb_rax_x86_64]=  0x;
+  reg_values[lldb_rbx_x86_64]=  0x;
+  reg_values[lldb_rcx_x86_64]=  0x0010;
+  reg_values[lldb_rdx_x86_64]=  0x;
+  reg_values[lldb_rdi_x86_64]=  0x7ffceb349cf0;
+  reg_values[lldb_rsi_x86_64]=  0x;
+  reg_values[lldb_rbp_x86_64]=  0x7ffceb34a210;
+  reg_values[lldb_rsp_x86_64]=  0x7ffceb34a210;
+  reg_values[lldb_r8_x86_64] =  0x7fe9bc1aa9c0;
+  reg_values[lldb_r9_x86_64] =  0x;
+  reg_values[lldb_r10_x86_64]=  0x7fe9bc3f16a0;
+  reg_values[lldb_r11_x86_64]=  0x0246;
+  reg_values[lldb_r12_x86_64]=  0x00401c92;
+  reg_values[lldb_r13_x86_64]=  0x7ffceb34a430;
+  reg_values[lldb_r14_x86_64]=  0x;
+  reg_values[lldb_r15_x86_64]=  0x;
+  reg_values[lldb_rip_x86_64]=  0x00401dc6;
+  reg_values[lldb_rflags_x86_64] =  0x00010206;
+  reg_values[lldb_cs_x86_64] =  0x0033;
+  reg_values[lldb_fs_x86_64] =  0x;
+  reg_values[lldb_gs_x86_64] =  0x;
+  reg_values[lldb_ss_x86_64] =  0x;
+  reg_values[lldb_ds_x86_64] =  0x;
+  reg_values[lldb_es_x86_64] =  0x;
+  // clang-format on
+
+  for (uint32_t reg_index = 0; reg_index < reg_interface->GetRegisterCount();
+   ++reg_index) {
+if (reg_values.find(reg_index) != reg_values.end()) {
+  registerEqualToVal(reg_values[reg_index],
+ buf->GetBytes() + reg_info[reg_index].byte_offset);
+}
+  }
+}
Index: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
===
--- /dev/null
+++ source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -0,0 +1,189 @@
+//===-- Registers_x86_64.h --*- C++ -*-===//
+//
+//

[Lldb-commits] [lldb] r282408 - Remove ancient icc decorators

2016-09-26 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Mon Sep 26 09:34:02 2016
New Revision: 282408

URL: http://llvm.org/viewvc/llvm-project?rev=282408=rev
Log:
Remove ancient icc decorators

Nobody is running the test suite with icc, so we have no idea if they pass. But
the bug they link to has definitely been fixed.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/TestDataFormatterStdIterator.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/TestDataFormatterStdIterator.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/TestDataFormatterStdIterator.py?rev=282408=282407=282408=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/TestDataFormatterStdIterator.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/TestDataFormatterStdIterator.py
 Mon Sep 26 09:34:02 2016
@@ -24,9 +24,6 @@ class StdIteratorDataFormatterTestCase(T
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
 @skipIfWindows  # libstdcpp not ported to Windows
-@expectedFailureAll(
-compiler="icc",
-bugnumber="llvm.org/pr15301 LLDB prints incorrect sizes of STL 
containers")
 def test_with_run_command(self):
 """Test that libstdcpp iterators format properly."""
 self.build()

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py?rev=282408=282407=282408=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py
 Mon Sep 26 09:34:02 2016
@@ -23,9 +23,6 @@ class StdMapDataFormatterTestCase(TestBa
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@expectedFailureAll(
-compiler="icc",
-bugnumber="llvm.org/pr15301 LLDB prints incorrect sizes of STL 
containers")
 @skipIfWindows  # libstdcpp not ported to Windows
 @skipIfFreeBSD
 def test_with_run_command(self):

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py?rev=282408=282407=282408=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
 Mon Sep 26 09:34:02 2016
@@ -26,9 +26,6 @@ class StdVBoolDataFormatterTestCase(Test
 @expectedFailureAll(
 oslist=['freebsd'],
 bugnumber='llvm.org/pr20548 fails to build on lab.llvm.org buildbot')
-@expectedFailureAll(
-compiler="icc",
-bugnumber="llvm.org/pr15301 LLDB prints incorrect sizes of STL 
containers")
 @skipIfWindows  # libstdcpp not ported to Windows.
 @skipIfDarwin
 def test_with_run_command(self):

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py?rev=282408=282407=282408=diff
==
---

[Lldb-commits] [lldb] r282406 - Remove an ancient XFAIL from TestBuiltinTrap

2016-09-26 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Mon Sep 26 08:50:06 2016
New Revision: 282406

URL: http://llvm.org/viewvc/llvm-project?rev=282406=rev
Log:
Remove an ancient XFAIL from TestBuiltinTrap

in refers to gcc-4.6. Hopefully noone is using that anymore, and I think there 
is
a good chance it was fixed anyway.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/linux/builtin_trap/TestBuiltinTrap.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/linux/builtin_trap/TestBuiltinTrap.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/linux/builtin_trap/TestBuiltinTrap.py?rev=282406=282405=282406=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/linux/builtin_trap/TestBuiltinTrap.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/linux/builtin_trap/TestBuiltinTrap.py 
Mon Sep 26 08:50:06 2016
@@ -23,12 +23,6 @@ class BuiltinTrapTestCase(TestBase):
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@expectedFailureAll(
-"llvm.org/pr15936",
-compiler="gcc",
-compiler_version=[
-"<=",
-"4.6"])
 # gcc generates incorrect linetable
 @expectedFailureAll(archs="arm", compiler="gcc", triple=".*-android")
 @expectedFailureAll(oslist=['linux'], archs=['arm'])


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

Re: [Lldb-commits] [PATCH] D24890: implement timeout sample support for Linux

2016-09-26 Thread Pavel Labath via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

lgtm


https://reviews.llvm.org/D24890



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

Re: [Lldb-commits] [PATCH] D24629: Allow for tests to be disabled at runtime

2016-09-26 Thread Pavel Labath via lldb-commits 
labath added a comment.

In https://reviews.llvm.org/D24629#550841, @fjricci wrote:

> In https://reviews.llvm.org/D24629#550823, @tfiala wrote:
>
> > > > There is no reasonable thing we can base the expectation as the exact 
> > > > same device with a different cpu revision could support watchpoints 
> > > > just fine, so we could just define the list of these tests externally 
> > > > (in this case, I would probably annotate them with the watchpoint 
> > > > category and then do the skips based on categories instead).
> >
> > > 
> >
> >
> > Tangential: most chips I've worked on that had hardware watchpoint support 
> > had an instruction that could be called to find out if such a feature 
> > exists.  I think ARM does this.  I would think we could expose an API that 
> > says whether watchpoints are supported or not, and use that info in LLDB 
> > and the test suite to enable or disable them.
>
>
> I believe that PTRACE_GETHBPREGS with a value of 0 returns the hardware 
> stoppoint info on arm, and the byte representing the number of available 
> hardware watchpoints will be 0 if they aren't supported. Not sure if there's 
> a simpler way.


It's a bit trickier than that. In some cases that call will still return 
non-zero as the number of supported watchpoints, but the "watchpoint size" 
field will be zero, and it will still mean that watchpoints don't work. This is 
probably a kernel bug, though it is pretty easy to work around. The more boring 
part would be plumbing that information all the way to the test suite - Nothing 
that can't be done, it's just a bit laborious, so I haven't done that yet.


Repository:
  rL LLVM

https://reviews.llvm.org/D24629



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