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

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

I don't think this is a totally bad idea. In fact we already had something like 
this (nobody used it though), before it was removed in  
https://reviews.llvm.org/rL255040. If it goes in, we might start using it 
actually -- e.g., currently we have watchpoint tests which fail on some devices 
which do not support watchpoints. 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).

That said, I do have slightly mixed feelings about it, as it is increasing the 
complexity of an already complex system, and there are other possible ways to 
solve the watchpoint problem (have the tests detect whether the device supports 
watchpoints, and self-skip when appropriate).



Comment at: packages/Python/lldbsuite/test/dotest.py:803
@@ +802,3 @@
+if configuration.skip_files:
+import re
+for file_regexp in configuration.skip_files:

We should just `import re` at top level. A lot of tests already do that, so 
it's not likely it will break anyone.


https://reviews.llvm.org/D24629



___
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-16 Thread Muhammad Omair Javaid via lldb-commits
omjavaid added a comment.

comments inline.



Comment at: 
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:43
@@ -42,2 +42,3 @@
 """Test to selectively watch different bytes in a 8-byte array."""
-self.run_watchpoint_size_test('byteArray', 8, '1')
+if self.getArchitecture() in ['arm']:
+self.run_watchpoint_size_test('byteArray', 8, '1', 1)

labath wrote:
> It's not clear to me why you need to modify the existing test for this 
> change. You are adding functionality, so all existing tests should pass as-is 
> (which will also validate that your change did not introduce regressions).
As we keep on adding these tests its increasing our overall testing time. I 
just thought using the same test with some changes will cover the cases we want 
to test.

Anyways we can write separate tests as well.


Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:556
@@ +555,3 @@
+uint32_t current_size = GetWatchpointSize(wp_index);
+if ((current_size == 4) || (current_size == 2 && watch_mask <= 2) ||
+(current_size == 1 && watch_mask == 1))

labath wrote:
> The logic here is extremely convoluted. Doesn't this code basically boil down 
> to:
> ```
> current_size = m_hwp_regs[wp_index].control & 1 ? GetWatchpointSize(wp_index) 
> : 0;
> new_size = llvm::NextPowerOf2(std::max(current_size, watch_mask));
> // update the control value, write the debug registers...
> ```
> 
> Also `watch_mask` should probably be renamed to `watch_size`, as it doesn't 
> appear to be a mask.
Seems legit. I ll update this in next patch.


Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:602
@@ -612,3 +601,3 @@
 
 bool NativeRegisterContextLinux_arm::ClearHardwareWatchpoint(
 uint32_t wp_index) {

labath wrote:
> This looks a bit worrying.
> What will happen after the following sequence of events:
> - client tells us to set a watchpoint at 0x1000
> - we set the watchpoint
> - client tells us to set a watchpoint at 0x1001
> - we extend the previous watchpoint to watch this address as well
> - client tells us to delete the watchpoint at 0x1000
> - ???
> 
> Will we remain watching the address 0x1001? I don't see how will you be able 
> to do that without maintaining a some info about the original watchpoints the 
> client requested (and I have not seen that code). Please add a test for this.
I just realized that I missed a crucial change in my last patch that we need to 
make all this work.

Let me get back with correction and desired test cases.


https://reviews.llvm.org/D24610



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


[Lldb-commits] [lldb] r281717 - [RenderScript] Support tracking and dumping reduction kernels

2016-09-16 Thread Luke Drummond via lldb-commits
Author: ldrumm
Date: Fri Sep 16 06:28:12 2016
New Revision: 281717

URL: http://llvm.org/viewvc/llvm-project?rev=281717&view=rev
Log:
[RenderScript] Support tracking and dumping reduction kernels

Initial implementation of support for tracking
[RenderScript 
Reductions](https://developer.android.com/guide/topics/renderscript/compute.html#reduction-in-depth)

With this patch, `language renderscript module dump` properly lists reductions
that are part of loaded RenderScript modules as well the the consituent
functions and their roles, within the reduction.

This support required new tracking mechanisms for the `#pragma(reduce)`
mechanism, and extension of `RSModuleDescriptor::ParseRSInfo` to support
the metadata output by `bcc`. This work was also an opportunity to
refactor/improve parse code:

- `RSModuleDescriptor::ParseExportReduceCount` now has a complete
  implementation and the debugger can correctly track reductions on
  receipt of a module hook.
- `RSModuleDescriptor::Dump` now dumps Reductions as well as `ForEach`
  kernels. Also, fixed indentation of the output, and made indentation
  groupings in the source clearer.
- `RSModuleDescriptor::ParseRSInfo` now returns true if the `".rs.info"`
  packet has nonzero linecount, rather than rejecting RenderScripts that
  don't contain kernels (an unlikely situation, but possibly valid). This
  was changed because scripts that only contained reductions were not
  being tracked in `RenderScriptRuntime::LoadModule`.
- Refactor `RSModuleInfo::ParseRSInfo` and add reduction spec parser stub
 - Prepared ParseRSInfo to more easily be able to add new parser types
 - Use llvm::StringRef and llvm::StringMap helpers to make the parsing code 
cleaner
 - factor out forEachCount, globalVarCount, and pragmaCount parsing block to 
their own methods
 - Add ExportReduceCount Parser
 - Use `llvm::StringRef` in `RSKernelDescriptor` constructor
 - removed now superfluous `MAXLINE` macros as we've switched from `const
   char *` to `llvm::StringRef`

Modified:

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp?rev=281717&r1=281716&r2=281717&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
 Fri Sep 16 06:28:12 2016
@@ -10,6 +10,8 @@
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
+#include "llvm/ADT/StringMap.h"
+
 // Project includes
 #include "RenderScriptRuntime.h"
 
@@ -2587,17 +2589,104 @@ void RenderScriptRuntime::Update() {
   }
 }
 
-// The maximum line length of an .rs.info packet
-#define MAXLINE 500
-#define STRINGIFY(x) #x
-#define MAXLINESTR_(x) "%" STRINGIFY(x) "s"
-#define MAXLINESTR MAXLINESTR_(MAXLINE)
+bool RSModuleDescriptor::ParsePragmaCount(llvm::StringRef *lines,
+  size_t n_lines) {
+  // Skip the pragma prototype line
+  ++lines;
+  for (; n_lines--; ++lines) {
+const auto kv_pair = lines->split(" - ");
+m_pragmas[kv_pair.first.trim().str()] = kv_pair.second.trim().str();
+  }
+  return true;
+}
+
+bool RSModuleDescriptor::ParseExportReduceCount(llvm::StringRef *lines,
+size_t n_lines) {
+  // The list of reduction kernels in the `.rs.info` symbol is of the form
+  // "signature - accumulatordatasize - reduction_name - initializer_name -
+  // accumulator_name - combiner_name -
+  // outconverter_name - halter_name"
+  // Where a function is not explicitly named by the user, or is not generated
+  // by the compiler, it is named "." so the
+  // dash separated list should always be 8 items long
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
+  // Skip the exportReduceCount line
+  ++lines;
+  for (; n_lines--; ++lines) {
+llvm::SmallVector spec;
+lines->split(spec, " - ");
+if (spec.size() != 8) {
+  if (spec.size() < 8) {
+if (log)
+  log->Error("Error parsing RenderScript reduction spec. wrong number "
+ "of fields");
+return false;
+  } else if (log)
+log->Warning("Extraneous members in reduction spec: '%s'",
+ lines->str().c_str());
+}
+
+const auto sig_s = spec[0];
+uint32_t sig;
+if (sig_s.getAsInteger(10, sig)) {
+  if (log)
+log->Error("Error parsing Renderscript reduction spec: invalid kernel "
+   "signature: '%s'",
+  

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

2016-09-16 Thread Muhammad Omair Javaid via lldb-commits
omjavaid updated this revision to Diff 71633.
omjavaid added a comment.
Herald added subscribers: srhines, danalbert, tberghammer.

I have added a new test case that tests suggested scnario without changing any 
previous test cases.

Also I have made sure we re validate all watchpoint installed on thread resume 
to make sure we have the latest values assigned to hardware watchpoint 
registers.

This passes on ARM (RaspberryPi3, Samsung Chromebook). I have not yet tested on 
android.

This will fail on targets which dont support multiple watchpoint slots.

Also this should fail on AArch64 which I am currently working on.


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,6 +198,9 @@
   m_stop_info.reason = StopReason::eStopReasonNone;
   m_stop_description.clear();
 
+  // Invalidate watchpoint index map to re-sync watchpoint registers.
+  m_watchpoint_index_map.clear();
+
   // If watchpoints have been set, but none on this thread,
   // then this is a new thread. So set all existing watchpoints.
   if (m_watchpoint_index_map.empty()) {
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -410,15 +410,13 @@
   if ((m_hbr_regs[bp_index].control & 1) == 0) {
 m_hbr_regs[bp_index].address = addr;
 m_hbr_regs[bp_index].control = control_value;
-m_hbr_regs[bp_index].refcount = 1;
 
 // PTRACE call to set corresponding hardware breakpoint register.
 error = WriteHardwareDebugRegs(eDREGTypeBREAK, bp_index);
 
 if (error.Fail()) {
   m_hbr_regs[bp_index].address = 0;
   m_hbr_regs[bp_index].control &= ~1;
-  m_hbr_regs[bp_index].refcount = 0;
 
   return LLDB_INVALID_INDEX32;
 }
@@ -508,8 +506,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 +535,58 @@
 return LLDB_INVALID_INDEX32;
   }
 
-  // Can't watch zero bytes
-  // Can't watch more than 4 bytes per WVR/WCR pair
-
-  if (size == 0 || size > 4)
-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);
+  // 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) {
+  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
+}
   }
 
-  // We can only watch up to four bytes that follow a 4 byte aligned address
-  // per watchpoint register pair, so make sure we can properly encode this.
-  addr_word_offset = addr % 4;
-  byte_mask = ((1u << size) - 1u) << addr_word_offset;
-
-  // Check if we need multiple watchpoint register
-  if (byte_mask > 0xfu)
+  // No vaccant slot available and no duplicate slot found.
+  if (wp_index == LLDB_INVALID_INDEX32)
 return LLDB_INVALID_INDEX32;
 
+  uint8_t current_watch_size, new_watch_size;
+  // Calculate overall size width to be watched by current hardware watchpoint slot.
+  current_watch_

Re: [Lldb-commits] [lldb] r281717 - [RenderScript] Support tracking and dumping reduction kernels

2016-09-16 Thread Zachary Turner via lldb-commits
On Fri, Sep 16, 2016 at 4:36 AM Luke Drummond via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: ldrumm
> Date: Fri Sep 16 06:28:12 2016
> New Revision: 281717
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281717&view=rev
> Log:
> [RenderScript] Support tracking and dumping reduction kernels
>
> Initial implementation of support for tracking
> [RenderScript Reductions](
> https://developer.android.com/guide/topics/renderscript/compute.html#reduction-in-depth
> )
>
> With this patch, `language renderscript module dump` properly lists
> reductions
> that are part of loaded RenderScript modules as well the the consituent
> functions and their roles, within the reduction.
>
> This support required new tracking mechanisms for the `#pragma(reduce)`
> mechanism, and extension of `RSModuleDescriptor::ParseRSInfo` to support
> the metadata output by `bcc`. This work was also an opportunity to
> refactor/improve parse code:
>
> - `RSModuleDescriptor::ParseExportReduceCount` now has a complete
>   implementation and the debugger can correctly track reductions on
>   receipt of a module hook.
> - `RSModuleDescriptor::Dump` now dumps Reductions as well as `ForEach`
>   kernels. Also, fixed indentation of the output, and made indentation
>   groupings in the source clearer.
> - `RSModuleDescriptor::ParseRSInfo` now returns true if the `".rs.info"`
>   packet has nonzero linecount, rather than rejecting RenderScripts that
>   don't contain kernels (an unlikely situation, but possibly valid). This
>   was changed because scripts that only contained reductions were not
>   being tracked in `RenderScriptRuntime::LoadModule`.
> - Refactor `RSModuleInfo::ParseRSInfo` and add reduction spec parser stub
>  - Prepared ParseRSInfo to more easily be able to add new parser types
>  - Use llvm::StringRef and llvm::StringMap helpers to make the parsing
> code cleaner
>  - factor out forEachCount, globalVarCount, and pragmaCount parsing block
> to their own methods
>  - Add ExportReduceCount Parser
>  - Use `llvm::StringRef` in `RSKernelDescriptor` constructor
>  - removed now superfluous `MAXLINE` macros as we've switched from `const
>char *` to `llvm::StringRef`
>
> Modified:
>
> lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
>
> lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
>
> Modified:
> lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp?rev=281717&r1=281716&r2=281717&view=diff
>
> ==
> ---
> lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
> (original)
> +++
> lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
> Fri Sep 16 06:28:12 2016
> @@ -10,6 +10,8 @@
>  // C Includes
>  // C++ Includes
>  // Other libraries and framework includes
> +#include "llvm/ADT/StringMap.h"
> +
>  // Project includes
>  #include "RenderScriptRuntime.h"
>
> @@ -2587,17 +2589,104 @@ void RenderScriptRuntime::Update() {
>}
>  }
>
> -// The maximum line length of an .rs.info packet
> -#define MAXLINE 500
> -#define STRINGIFY(x) #x
> -#define MAXLINESTR_(x) "%" STRINGIFY(x) "s"
> -#define MAXLINESTR MAXLINESTR_(MAXLINE)
> +bool RSModuleDescriptor::ParsePragmaCount(llvm::StringRef *lines,
> +  size_t n_lines) {
> +  // Skip the pragma prototype line
> +  ++lines;
> +  for (; n_lines--; ++lines) {
> +const auto kv_pair = lines->split(" - ");
> +m_pragmas[kv_pair.first.trim().str()] = kv_pair.second.trim().str();
> +  }
> +  return true;
> +}
> +
> +bool RSModuleDescriptor::ParseExportReduceCount(llvm::StringRef *lines,
> +size_t n_lines) {
>
This should take an ArrayRef.  In general this is true any time
you're passing an array to a function via (T* items, size_t length).


> +  // The list of reduction kernels in the `.rs.info` symbol is of the
> form
> +  // "signature - accumulatordatasize - reduction_name - initializer_name
> -
> +  // accumulator_name - combiner_name -
> +  // outconverter_name - halter_name"
> +  // Where a function is not explicitly named by the user, or is not
> generated
> +  // by the compiler, it is named "." so the
> +  // dash separated list should always be 8 items long
> +  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
> +  // Skip the exportReduceCount line
> +  ++lines;
> +  for (; n_lines--; ++lines) {
>
With ArrayRef this would be written as:

for (StringRef line : lines.drop_front()) {
}


> +llvm::SmallVector spec;
> +lines->split(spec, " - ");
> +if (spec.size() != 8) {
> +  if (spec.size() < 8) {
> +i

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

2016-09-16 Thread Francis Ricci via lldb-commits
fjricci updated this revision to Diff 71651.
fjricci added a comment.

Refactor re


https://reviews.llvm.org/D24629

Files:
  packages/Python/lldbsuite/test/configuration.py
  packages/Python/lldbsuite/test/dotest.py
  packages/Python/lldbsuite/test/dotest_args.py
  packages/Python/lldbsuite/test/test_result.py

Index: packages/Python/lldbsuite/test/test_result.py
===
--- packages/Python/lldbsuite/test/test_result.py
+++ packages/Python/lldbsuite/test/test_result.py
@@ -18,6 +18,8 @@
 # Third-party modules
 import unittest2
 
+from unittest2.util import strclass
+
 # LLDB Modules
 from . import configuration
 from lldbsuite.test_event.event_builder import EventBuilder
@@ -124,10 +126,23 @@
 test,
 test._testMethodName).__func__.__unittest_skip_why__ = "test case does not fall in any category of interest for this run"
 
+def checkExclusion(self, exclusion_list, name):
+if exclusion_list:
+import re
+for item in exclusion_list:
+if re.search(item, name):
+return True
+return False
+
 def startTest(self, test):
 if configuration.shouldSkipBecauseOfCategories(
 self.getCategoriesForTest(test)):
 self.hardMarkAsSkipped(test)
+if self.checkExclusion(
+configuration.skip_methods,
+test._testMethodName):
+self.hardMarkAsSkipped(test)
+
 configuration.setCrashInfoHook(
 "%s at %s" %
 (str(test), inspect.getfile(
@@ -145,6 +160,15 @@
 EventBuilder.event_for_start(test))
 
 def addSuccess(self, test):
+if self.checkExclusion(
+configuration.xfail_files,
+strclass(
+test.__class__)) or self.checkExclusion(
+configuration.xfail_methods,
+test._testMethodName):
+self.addUnexpectedSuccess(test, None)
+return
+
 super(LLDBTestResult, self).addSuccess(test)
 if configuration.parsable:
 self.stream.write(
@@ -214,6 +238,15 @@
 test, err))
 
 def addFailure(self, test, err):
+if self.checkExclusion(
+configuration.xfail_files,
+strclass(
+test.__class__)) or self.checkExclusion(
+configuration.xfail_methods,
+test._testMethodName):
+self.addExpectedFailure(test, err, None)
+return
+
 configuration.sdir_has_content = True
 super(LLDBTestResult, self).addFailure(test, err)
 method = getattr(test, "markFailure", None)
Index: packages/Python/lldbsuite/test/dotest_args.py
===
--- packages/Python/lldbsuite/test/dotest_args.py
+++ packages/Python/lldbsuite/test/dotest_args.py
@@ -96,6 +96,9 @@
 '-p',
 metavar='pattern',
 help='Specify a regexp filename pattern for inclusion in the test suite')
+group.add_argument('--excluded', metavar='exclusion-file', help=textwrap.dedent(
+'''Specify a file for tests to exclude. File should contain lists of regular expressions for test files or methods,
+with each list under a matching header (xfail files, xfail methods, skip files, skip methods)'''))
 group.add_argument(
 '-G',
 '--category',
Index: packages/Python/lldbsuite/test/dotest.py
===
--- packages/Python/lldbsuite/test/dotest.py
+++ packages/Python/lldbsuite/test/dotest.py
@@ -26,6 +26,7 @@
 import os
 import errno
 import platform
+import re
 import signal
 import socket
 import subprocess
@@ -202,6 +203,48 @@
 sys.exit(0)
 
 
+def parseExclusion(exclusion_file):
+"""Parse an exclusion file, of the following format, where
+   'skip files', 'skip methods', 'xfail files', and 'xfail methods'
+   are the possible list heading values:
+
+   skip files
+   
+   
+
+   xfail methods
+   
+"""
+excl_type = None
+case_type = None
+
+with open(exclusion_file) as f:
+for line in f:
+if not excl_type:
+[excl_type, case_type] = line.split()
+continue
+
+line = line.strip()
+if not line:
+excl_type = None
+elif excl_type == 'skip' and case_type == 'files':
+if not configuration.skip_files:
+configuration.skip_files = []
+configuration.skip_files.append(line)
+elif excl_type == 'skip' and case_type == 'methods':
+if not configuration.skip_methods:
+configuration.skip_methods = []
+configuration.skip_methods.append(line)
+elif excl_type == 'xfail' and

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

2016-09-16 Thread Francis Ricci via lldb-commits
fjricci added a comment.

I do understand the complexity problem, and it was one of my concerns with this 
as well. For my cases, the complexity here is significantly less than the 
alternatives, but I also do understand if you don't think that's generally true.

It probably comes down to how often we think that people are running the test 
suite in cases where this sort of functionality would be useful. I don't really 
have a good sense for how other people tend to use the test suite, so I'm 
personally not sure.  For our case, it's a big deal, but if we're the only 
people who this patch helps, I know it doesn't make sense to merge it.


https://reviews.llvm.org/D24629



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


Re: [Lldb-commits] [PATCH] D24283: [lldb] Set the correct triple when creating an ArchSpec for windows

2016-09-16 Thread walter erquinigo via lldb-commits
wallace added a comment.

ping


https://reviews.llvm.org/D24283



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


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

2016-09-16 Thread walter erquinigo via lldb-commits
wallace added a comment.

ping


https://reviews.llvm.org/D24284



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


Re: [Lldb-commits] [PATCH] D24283: [lldb] Set the correct triple when creating an ArchSpec for windows

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

Thanks for the reminder.  Will do this today.


https://reviews.llvm.org/D24283



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


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

2016-09-16 Thread Zachary Turner via lldb-commits
zturner accepted this revision.
This revision is now accepted and ready to land.


Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:89
@@ +88,3 @@
+  if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp)) {
+return NULL;
+  }

s/NULL/nullptr/


https://reviews.llvm.org/D24284



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


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

2016-09-16 Thread walter erquinigo via lldb-commits
wallace added a comment.

I'm obliged, your grace


https://reviews.llvm.org/D24284



___
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-16 Thread Zachary Turner via lldb-commits
zturner added a subscriber: zturner.


Comment at: 
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c:23
@@ +22,3 @@
+{
+printf("About to write byteArray[%d] ...\n", i); // About to write 
byteArray
+

What's up with all the double spaced source code?  Is this intentional?


Comment at: 
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:513-521
@@ -513,1 +512,11 @@
+
+  // 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);
 

This block of code is a bit confusing to me.  Is this equivalent to:

```
lldb::addr_t start = llvm::alignDown(addr, 4);
lldb::addr_t end = addr + size;
if (start == end || (end-start)>4)
  return LLDB_INVALID_INDEX32;
```


https://reviews.llvm.org/D24610



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


[Lldb-commits] [lldb] r281765 - Set the correct triple when creating an ArchSpec for Windows.

2016-09-16 Thread Zachary Turner via lldb-commits
Author: zturner
Date: Fri Sep 16 14:09:19 2016
New Revision: 281765

URL: http://llvm.org/viewvc/llvm-project?rev=281765&view=rev
Log:
Set the correct triple when creating an ArchSpec for Windows.

Patch by Walter Erquinigo
Differential Revision: https://reviews.llvm.org/D24283

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

Modified: lldb/trunk/source/Core/ArchSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ArchSpec.cpp?rev=281765&r1=281764&r2=281765&view=diff
==
--- lldb/trunk/source/Core/ArchSpec.cpp (original)
+++ lldb/trunk/source/Core/ArchSpec.cpp Fri Sep 16 14:09:19 2016
@@ -1030,6 +1030,9 @@ bool ArchSpec::SetArchitecture(Architect
 m_triple.setOS(llvm::Triple::OSType::Solaris);
 break;
   }
+} else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) {
+  m_triple.setVendor(llvm::Triple::PC);
+  m_triple.setOS(llvm::Triple::Win32);
 } else {
   m_triple.setVendor(llvm::Triple::UnknownVendor);
   m_triple.setOS(llvm::Triple::UnknownOS);


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


[Lldb-commits] [lldb] r281764 - Add unit tests for a few string conversion functions in Args.

2016-09-16 Thread Zachary Turner via lldb-commits
Author: zturner
Date: Fri Sep 16 14:09:12 2016
New Revision: 281764

URL: http://llvm.org/viewvc/llvm-project?rev=281764&view=rev
Log:
Add unit tests for a few string conversion functions in Args.

Also provided a StringRef overload for these functions and have
the const char* overloads delegate to the StringRef overload.

Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointID.h
lldb/trunk/include/lldb/Host/FileSpec.h
lldb/trunk/include/lldb/Interpreter/Args.h
lldb/trunk/include/lldb/Interpreter/Options.h
lldb/trunk/include/lldb/Target/Language.h
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/Interpreter/Args.cpp
lldb/trunk/source/Interpreter/OptionValueChar.cpp
lldb/trunk/source/Interpreter/Property.cpp
lldb/trunk/unittests/Interpreter/TestArgs.cpp

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointID.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointID.h?rev=281764&r1=281763&r2=281764&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointID.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointID.h Fri Sep 16 14:09:12 2016
@@ -87,6 +87,7 @@ public:
   /// \b true if the name is a breakpoint name (as opposed to an ID or
   /// range) false otherwise.
   //--
+  // TODO: Convert this function to use a StringRef.
   static bool StringIsBreakpointName(const char *name, Error &error);
 
   //--

Modified: lldb/trunk/include/lldb/Host/FileSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=281764&r1=281763&r2=281764&view=diff
==
--- lldb/trunk/include/lldb/Host/FileSpec.h (original)
+++ lldb/trunk/include/lldb/Host/FileSpec.h Fri Sep 16 14:09:12 2016
@@ -80,6 +80,7 @@ public:
   ///
   /// @see FileSpec::SetFile (const char *path, bool resolve)
   //--
+  // TODO: Convert this constructor to use a StringRef.
   explicit FileSpec(const char *path, bool resolve_path,
 PathSyntax syntax = ePathSyntaxHostNative);
 

Modified: lldb/trunk/include/lldb/Interpreter/Args.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=281764&r1=281763&r2=281764&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/Args.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Args.h Fri Sep 16 14:09:12 2016
@@ -179,6 +179,7 @@ public:
   /// @return
   /// The NULL terminated C string of the copy of \a arg_cstr.
   //--
+  // TODO: Convert this function to use a StringRef.
   const char *AppendArgument(const char *arg_cstr, char quote_char = '\0');
 
   void AppendArguments(const Args &rhs);
@@ -353,24 +354,26 @@ public:
 return min <= sval64 && sval64 <= max;
   }
 
+  // TODO: Make this function take a StringRef
   static lldb::addr_t StringToAddress(const ExecutionContext *exe_ctx,
   const char *s, lldb::addr_t fail_value,
   Error *error);
 
   static bool StringToBoolean(const char *s, bool fail_value,
-  bool *success_ptr);
+bool *success_ptr);
 
   static bool StringToBoolean(llvm::StringRef s, bool fail_value,
   bool *success_ptr);
 
-  static char StringToChar(const char *s, char fail_value, bool *success_ptr);
+  static char StringToChar(llvm::StringRef s, char fail_value,
+   bool *success_ptr);
 
   static int64_t StringToOptionEnum(const char *s,
 OptionEnumValueElement *enum_values,
 int32_t fail_value, Error &error);
 
   static lldb::ScriptLanguage
-  StringToScriptLanguage(const char *s, lldb::ScriptLanguage fail_value,
+  StringToScriptLanguage(llvm::StringRef s, lldb::ScriptLanguage fail_value,
  bool *success_ptr);
 
   // TODO: Use StringRef

Modified: lldb/trunk/include/lldb/Interpreter/Options.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Options.h?rev=281764&r1=281763&r2=281764&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/Options.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Options.h Fri Sep 16 14:09:12 2016
@@ -188,6 +188,7 @@ public:
   /// @see Args::ParseOptions (Options&)
   /// @see man getopt_long_only
   //--
+  // TODO: Make this function take a StringRef.
   virtual Error

Re: [Lldb-commits] [PATCH] D24283: [lldb] Set the correct triple when creating an ArchSpec for windows

2016-09-16 Thread Zachary Turner via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281765: Set the correct triple when creating an ArchSpec for 
Windows. (authored by zturner).

Changed prior to commit:
  https://reviews.llvm.org/D24283?vs=71226&id=71686#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24283

Files:
  lldb/trunk/source/Core/ArchSpec.cpp

Index: lldb/trunk/source/Core/ArchSpec.cpp
===
--- lldb/trunk/source/Core/ArchSpec.cpp
+++ lldb/trunk/source/Core/ArchSpec.cpp
@@ -1030,6 +1030,9 @@
 m_triple.setOS(llvm::Triple::OSType::Solaris);
 break;
   }
+} else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) {
+  m_triple.setVendor(llvm::Triple::PC);
+  m_triple.setOS(llvm::Triple::Win32);
 } else {
   m_triple.setVendor(llvm::Triple::UnknownVendor);
   m_triple.setOS(llvm::Triple::UnknownOS);


Index: lldb/trunk/source/Core/ArchSpec.cpp
===
--- lldb/trunk/source/Core/ArchSpec.cpp
+++ lldb/trunk/source/Core/ArchSpec.cpp
@@ -1030,6 +1030,9 @@
 m_triple.setOS(llvm::Triple::OSType::Solaris);
 break;
   }
+} else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) {
+  m_triple.setVendor(llvm::Triple::PC);
+  m_triple.setOS(llvm::Triple::Win32);
 } else {
   m_triple.setVendor(llvm::Triple::UnknownVendor);
   m_triple.setOS(llvm::Triple::UnknownOS);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r281770 - Fix compiler warnings where two values weren't being initialized.

2016-09-16 Thread Greg Clayton via lldb-commits
Author: gclayton
Date: Fri Sep 16 15:10:02 2016
New Revision: 281770

URL: http://llvm.org/viewvc/llvm-project?rev=281770&view=rev
Log:
Fix compiler warnings where two values weren't being initialized.

Modified:
lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h

Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h?rev=281770&r1=281769&r2=281770&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h (original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h Fri Sep 16 
15:10:02 2016
@@ -131,7 +131,7 @@
 LLVM_EXTENSION BNDR_OFFSET(i), eEncodingVector, eFormatVectorOfUInt64, 
\
 {dwarf_##reg##i##_i386, dwarf_##reg##i##_i386, LLDB_INVALID_REGNUM,
\
  LLDB_INVALID_REGNUM, lldb_##reg##i##_i386 },  
\
- NULL, NULL
\
+ NULL, NULL, NULL, 0   
\
   }
 
 #define DEFINE_BNDC(name, i)   
\
@@ -141,7 +141,7 @@
eFormatVectorOfUInt8,   
\
{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, 
\
 LLDB_INVALID_REGNUM, lldb_##name##_i386 }, 
\
-NULL, NULL 
\
+NULL, NULL, NULL, 0
\
   }
 
 #define DEFINE_DR(reg, i)  
\


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


[Lldb-commits] LLVM buildmaster will be updated and restarted tonight

2016-09-16 Thread Galina Kistanova via lldb-commits
Hello everyone,

LLVM buildmaster will be updated and restarted after 6 PM Pacific time
today.

Thanks

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


Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support

2016-09-16 Thread Jason Molenda via lldb-commits
I thought about this more overnight and I'm more convinced that lit and lldb-mi 
make a great pair.  lldb-mi is a programmatic text format that isn't subject to 
the whims of command-line UI design over the years; well tests written in terms 
of MI will be resilient and stable.  lldb-mi MUCH more closely matches a 
non-lldb developer's view of how a debugger works.  The syntax is different 
from what they use, but it isn't hard to figure out.  I haven't touched MI in 
four or five years and it only took me a couple minutes to figure out how to do 
run to a breakpoint and print a variable:

-file-exec-and-symbols "/tmp/a.out"

-break-insert a.c:10
=breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x00010f70",func="main",file="a.c",fullname="/tmp/a.c",line="10",times="0",original-location="a.c:10"}

-exec-run

-thread-info

-exec-next

-var-create var1 * c
^done,name="var1",numchild="0",value="12",type="int",thread-id="1",has_more="0"


(I omitted the responses from all the command except -break-insert and 
-var-create to make it easier to read, but go run lldb-mi and play with it 
yourself or read the gdb MI documentation on the web.  It's a really simple 
format to work in.)


Several of the proposal have been special commands in the lldb command line 
driver that have non-changing output styles or the like.  Why re-invent 
something that already exists?  This makes a ton more sense on every front.  
Yes, it means you can't copy and paste your lldb debug session into a test case 
--- but we've all agreed that that's not something we want anyone doing anyway.

I'm in favor of lit + lldb-mi and think this would add a lot of value as an 
additional way for people to write test cases.


J



> On Sep 15, 2016, at 7:40 PM, Zachary Turner  wrote:
> 
> One thing I wonder about. It seems like everyone is mostly on the same page 
> about command line output .
> 
> What about input? Would anyone have objections to a test which ran a few 
> commands to get the debugger into a particular state before doing something 
> to verify the output? Let's assume I'm waving my hands about this last step 
> but that it doesn't involve scraping the output of a debugger command 
> 
> Maybe this will never even be an issue, but I just want to make sure everyone 
> is on the same page and that the objections are:
> 
> a) specific to command OUTPUT, not input (i.e. it's ok to have a test run 
> "break set -n main")
> b) specific to *non trivial* output (checking that "p 5" displays 5 is ok)
> c) specific to the the output of the *user* command line interface, so that 
> some hypothetical other command line interface (which again I'm being hand 
> wavy about) would not be subject to the same objections.
> 
> 
> On Thu, Sep 15, 2016 at 7:28 PM Jason Molenda  wrote:
> 
> > On Sep 15, 2016, at 8:02 AM, Zachary Turner  wrote:
> >
> >
> > It sounds like your goal is also "tests have to use the SB API and no other 
> > API", which if so I think that's counterproductive.   More productive, IMO, 
> > would be being open to any alternative that addresses the concerns you have 
> > with command-line tests.  There are more than 2 ways to skin a cat, so to 
> > speak.
> 
> Thinking about this a bit, another approach would be to do lit tests on top 
> of lldb-mi.  MI is a structured format for the debugger and a UI to 
> communicate back and forth with a simple text markup language (it would be 
> JSON if it were being done today, but it was added to gdb eighteen years ago, 
> so it's not).  The commands correspond to the commands a debugger user would 
> think to use -- no need to understand the structure of how lldb is 
> implemented, like with the SB API.  The format is a little unusual for a 
> human to type, but back when we supported gdb at Apple we worked in MI all 
> the time (it was used to communicate between Xcode, our IDE, and gdb) by hand 
> when testing and it was fine. "-exec-run" instead of run, that kind of thing. 
>  I think there are four dozens different commands.
> 
> lldb-mi itself uses SB API.  And the concerns about hardcoding command line 
> UI don't apply, it's a key-value format intended for computers, no one is 
> going to add an extra space character to anything -- the most it changes is 
> that new key-value pairs are added to responses.
> 
> 
> I agree there are many acceptable ways to write lit tests that don't involve 
> lldb command line scraping, and I'm all in favor of using lit with tests that 
> don't do that.  Of course the patch we're discussing has lit test examples 
> that contradict our own policy on writing tests.  Once lit is supported in 
> lldb, are we going to reject any testsuite additions that depend on the text 
> output from the command line lldb?  If we're all on the same page here, then 
> I have no reservations.
> 
> Just to say out loud the future I can easily see:  We add lit, then we have 
> people helpfully write a few dozen tests in lit that dep

Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support

2016-09-16 Thread Zachary Turner via lldb-commits
I have been thinking about something similar. Ildb-mi specifically I have
serious concerns about because the code itself is a bit of an abomination.
It has its own entire test suite, which also doesn't work very well.

Using a tool like lldb mi is very similar in spirit to how llvm already
uses lit though. There is a collection of very specific tools that exist
just for testing, and the output of those is run through lit)

Lldb-mi was developed independently, had no review, and was essentially a
large code drop. and it also was not developed with testing in mind. I
agree we could probably make it work, but I've been in that code a few
times and I have to say, I don't want to go back.

In any case, i think we agree in principle that from a high level, we could
get a lot of benefits from this style of test.

I'll have more specifics on my thoughts in a few weeks


On Fri, Sep 16, 2016 at 4:07 PM Jason Molenda  wrote:

> I thought about this more overnight and I'm more convinced that lit and
> lldb-mi make a great pair.  lldb-mi is a programmatic text format that
> isn't subject to the whims of command-line UI design over the years; well
> tests written in terms of MI will be resilient and stable.  lldb-mi MUCH
> more closely matches a non-lldb developer's view of how a debugger works.
> The syntax is different from what they use, but it isn't hard to figure
> out.  I haven't touched MI in four or five years and it only took me a
> couple minutes to figure out how to do run to a breakpoint and print a
> variable:
>
> -file-exec-and-symbols "/tmp/a.out"
>
> -break-insert a.c:10
>
> =breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x00010f70",func="main",file="a.c",fullname="/tmp/a.c",line="10",times="0",original-location="a.c:10"}
>
> -exec-run
>
> -thread-info
>
> -exec-next
>
> -var-create var1 * c
>
> ^done,name="var1",numchild="0",value="12",type="int",thread-id="1",has_more="0"
>
>
> (I omitted the responses from all the command except -break-insert and
> -var-create to make it easier to read, but go run lldb-mi and play with it
> yourself or read the gdb MI documentation on the web.  It's a really simple
> format to work in.)
>
>
> Several of the proposal have been special commands in the lldb command
> line driver that have non-changing output styles or the like.  Why
> re-invent something that already exists?  This makes a ton more sense on
> every front.  Yes, it means you can't copy and paste your lldb debug
> session into a test case --- but we've all agreed that that's not something
> we want anyone doing anyway.
>
> I'm in favor of lit + lldb-mi and think this would add a lot of value as
> an additional way for people to write test cases.
>
>
> J
>
>
>
> > On Sep 15, 2016, at 7:40 PM, Zachary Turner  wrote:
> >
> > One thing I wonder about. It seems like everyone is mostly on the same
> page about command line output .
> >
> > What about input? Would anyone have objections to a test which ran a few
> commands to get the debugger into a particular state before doing something
> to verify the output? Let's assume I'm waving my hands about this last step
> but that it doesn't involve scraping the output of a debugger command
> >
> > Maybe this will never even be an issue, but I just want to make sure
> everyone is on the same page and that the objections are:
> >
> > a) specific to command OUTPUT, not input (i.e. it's ok to have a test
> run "break set -n main")
> > b) specific to *non trivial* output (checking that "p 5" displays 5 is
> ok)
> > c) specific to the the output of the *user* command line interface, so
> that some hypothetical other command line interface (which again I'm being
> hand wavy about) would not be subject to the same objections.
> >
> >
> > On Thu, Sep 15, 2016 at 7:28 PM Jason Molenda 
> wrote:
> >
> > > On Sep 15, 2016, at 8:02 AM, Zachary Turner 
> wrote:
> > >
> > >
> > > It sounds like your goal is also "tests have to use the SB API and no
> other API", which if so I think that's counterproductive.   More
> productive, IMO, would be being open to any alternative that addresses the
> concerns you have with command-line tests.  There are more than 2 ways to
> skin a cat, so to speak.
> >
> > Thinking about this a bit, another approach would be to do lit tests on
> top of lldb-mi.  MI is a structured format for the debugger and a UI to
> communicate back and forth with a simple text markup language (it would be
> JSON if it were being done today, but it was added to gdb eighteen years
> ago, so it's not).  The commands correspond to the commands a debugger user
> would think to use -- no need to understand the structure of how lldb is
> implemented, like with the SB API.  The format is a little unusual for a
> human to type, but back when we supported gdb at Apple we worked in MI all
> the time (it was used to communicate between Xcode, our IDE, and gdb) by
> hand when testing and it was fine. "-exec-run" instead of run, that kin

[Lldb-commits] [PATCH] D24694: [LLDB] Fix Clang initialization and Clang-tidy modernize-use-nullptr warnings

2016-09-16 Thread Eugene Zelenko via lldb-commits
Eugene.Zelenko created this revision.
Eugene.Zelenko added reviewers: labath, zturner.
Eugene.Zelenko added a subscriber: lldb-commits.
Eugene.Zelenko set the repository for this revision to rL LLVM.

Looks like RegisterContextNetBSD_x86_64.cpp should use RegisterInfos_x86_64.h 
as much as possible, but I think will be good idea if platform maintainer will 
take care about this.


Repository:
  rL LLVM

https://reviews.llvm.org/D24694

Files:
  source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp
  source/Plugins/Process/Utility/RegisterInfos_i386.h
  source/Plugins/Process/Utility/RegisterInfos_x86_64.h

Index: source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp
===
--- source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp
+++ source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp
@@ -1,15 +1,16 @@
-//===-- RegisterContextNetBSD_x86_64.cpp --*- C++ -*-===//
+//===-- RegisterContextNetBSD_x86_64.cpp *- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//===-===//
+//===--===//
 
+#include 
 #include 
-#include 
 
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/Compiler.h"
 
 #include "RegisterContextNetBSD_x86_64.h"
@@ -89,10 +90,10 @@
LLVM_EXTENSION offsetof(XSAVE, ymmh[0]) + (32 * reg_index))
 
 // Number of bytes needed to represent a FPR.
-#define FPR_SIZE(reg) sizeof(((FXSAVE *)NULL)->reg)
+#define FPR_SIZE(reg) sizeof(((FXSAVE *)nullptr)->reg)
 
 // Number of bytes needed to represent the i'th FP register.
-#define FP_SIZE sizeof(((MMSReg *)NULL)->bytes)
+#define FP_SIZE sizeof(((MMSReg *)nullptr)->bytes)
 
 // Number of bytes needed to represent an XMM register.
 #define XMM_SIZE sizeof(XMMReg)
@@ -105,101 +106,106 @@
 // Note that the size and offset will be updated by platform-specific classes.
 #define DEFINE_GPR(reg, alt, kind1, kind2, kind3, kind4)   \
   {\
-#reg, alt, sizeof(((GPR *) NULL)->reg),\
+#reg, alt, sizeof(((GPR *)nullptr)->reg),  \
   GPR_OFFSET(reg), eEncodingUint, eFormatHex,  \
  {kind1, kind2, kind3, kind4,  \
   lldb_##reg##_x86_64 },   \
-  NULL, NULL   \
+  nullptr, nullptr, nullptr, 0 \
   }
 
 #define DEFINE_FPR(name, reg, kind1, kind2, kind3, kind4)  \
   {\
-#name, NULL, FPR_SIZE(reg), FPR_OFFSET(reg), eEncodingUint, eFormatHex,\
+#name, nullptr, FPR_SIZE(reg), FPR_OFFSET(reg), eEncodingUint, eFormatHex, \
{kind1, kind2, kind3, kind4,\
 lldb_##name##_x86_64 },\
-NULL, NULL \
+nullptr, nullptr, nullptr, 0   \
   }
 
 #define DEFINE_FP_ST(reg, i)   \
   {\
-#reg #i, NULL, FP_SIZE,\
+#reg #i, nullptr, FP_SIZE, \
 LLVM_EXTENSION FPR_OFFSET( \
 stmm[i]), eEncodingVector, eFormatVectorOfUInt8,   \
 {dwarf_st##i##_x86_64, dwarf_st##i##_x86_64, LLDB_INVALID_REGNUM,  \
  LLDB_INVALID_REGNUM, lldb_st##i##_x86_64 },   \
- NULL, NULL\
+ nullptr, nullptr, nullptr, 0  \
   }
 
 #define DEFINE_FP_MM(reg, i)   \
   {\
-#reg #i, NULL, sizeof(uint64_t),   \
+#reg #i, nullptr, sizeof(uint64_t),\
   LLVM_EXTENSION FPR_OFFSET(   \
   stmm[i]), eEncodingUint, eFormatHex, \
   {dwarf_mm##i##_x86_64, dwarf_mm##i##_x86_64, \

[Lldb-commits] [lldb] r281799 - Convert many functions to use StringRefs.

2016-09-16 Thread Zachary Turner via lldb-commits
Author: zturner
Date: Fri Sep 16 21:00:02 2016
New Revision: 281799

URL: http://llvm.org/viewvc/llvm-project?rev=281799&view=rev
Log:
Convert many functions to use StringRefs.

Where possible, remove the const char* version.  To keep the
risk and impact here minimal, I've only done the simplest
functions.

In the process, I found a few opportunities for adding some
unit tests, so I added those as well.

Tested on Windows, Linux, and OSX.

Added:
lldb/trunk/unittests/Breakpoint/
lldb/trunk/unittests/Breakpoint/BreakpointIDTest.cpp
lldb/trunk/unittests/Breakpoint/CMakeLists.txt
lldb/trunk/unittests/Platform/
lldb/trunk/unittests/Platform/CMakeLists.txt
lldb/trunk/unittests/Platform/PlatformDarwinTest.cpp
Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointID.h
lldb/trunk/include/lldb/Interpreter/Args.h
lldb/trunk/include/lldb/Target/Language.h
lldb/trunk/source/Breakpoint/Breakpoint.cpp
lldb/trunk/source/Breakpoint/BreakpointID.cpp
lldb/trunk/source/Breakpoint/BreakpointIDList.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Interpreter/Args.cpp
lldb/trunk/source/Interpreter/OptionGroupPlatform.cpp
lldb/trunk/source/Interpreter/OptionValueLanguage.cpp
lldb/trunk/source/Interpreter/Property.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Target/Language.cpp
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/Interpreter/TestArgs.cpp

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointID.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointID.h?rev=281799&r1=281798&r2=281799&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointID.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointID.h Fri Sep 16 21:00:02 2016
@@ -17,6 +17,8 @@
 
 #include "lldb/lldb-private.h"
 
+#include "llvm/ADT/StringRef.h"
+
 namespace lldb_private {
 
 //--
@@ -87,8 +89,7 @@ public:
   /// \b true if the name is a breakpoint name (as opposed to an ID or
   /// range) false otherwise.
   //--
-  // TODO: Convert this function to use a StringRef.
-  static bool StringIsBreakpointName(const char *name, Error &error);
+  static bool StringIsBreakpointName(llvm::StringRef str, Error &error);
 
   //--
   /// Takes a breakpoint ID and the breakpoint location id and returns

Modified: lldb/trunk/include/lldb/Interpreter/Args.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=281799&r1=281798&r2=281799&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/Args.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Args.h Fri Sep 16 21:00:02 2016
@@ -394,9 +394,8 @@ public:
 
   static uint32_t StringToGenericRegister(llvm::StringRef s);
 
-  // TODO: Update to take a StringRef
-  static const char *StringToVersion(const char *s, uint32_t &major,
- uint32_t &minor, uint32_t &update);
+  static bool StringToVersion(llvm::StringRef string, uint32_t &major,
+  uint32_t &minor, uint32_t &update);
 
   static const char *GetShellSafeArgument(const FileSpec &shell,
   const char *unsafe_arg,

Modified: lldb/trunk/include/lldb/Target/Language.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Language.h?rev=281799&r1=281798&r2=281799&view=diff
==
--- lldb/trunk/include/lldb/Target/Language.h (original)
+++ lldb/trunk/include/lldb/Target/Language.h Fri Sep 16 21:00:02 2016
@@ -142,8 +142,8 @@ public:
   // These are accessors for general information about the Languages lldb knows
   // about:
 
-  // TODO: Convert this to using a StringRef.
   static lldb::LanguageType GetLanguageTypeFromString(const char *string);
+  static lldb::LanguageType GetLanguageTypeFromString(llvm::StringRef string);
 
   static const char *GetNameForLanguageType(lldb::LanguageType language);
 

Modified: lldb/trunk/source/Breakpoint/Breakpoint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Breakpoint.cpp?rev=281799&r1=281798&r2=281799&view

[Lldb-commits] [lldb] r281804 - Fix boolean logic error in BreakpointID.

2016-09-16 Thread Zachary Turner via lldb-commits
Author: zturner
Date: Fri Sep 16 21:34:40 2016
New Revision: 281804

URL: http://llvm.org/viewvc/llvm-project?rev=281804&view=rev
Log:
Fix boolean logic error in BreakpointID.

Modified:
lldb/trunk/source/Breakpoint/BreakpointID.cpp

Modified: lldb/trunk/source/Breakpoint/BreakpointID.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointID.cpp?rev=281804&r1=281803&r2=281804&view=diff
==
--- lldb/trunk/source/Breakpoint/BreakpointID.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointID.cpp Fri Sep 16 21:34:40 2016
@@ -113,7 +113,7 @@ bool BreakpointID::StringIsBreakpointNam
 return false;
 
   // First character must be a letter or _
-  if (!isalpha(str[0]) || str[0] != '_')
+  if (!isalpha(str[0]) && str[0] != '_')
 return false;
 
   // Cannot contain ., -, or space.


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