[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#912810, @clayborg wrote:

> I was unhappy when we went over two pointers for a FileSpec when m_syntax was 
> added due to the extra size. Anything we can do to make this smaller would be 
> great, so the type on the enum would work, but as you say the alignment will 
> nullify that. The two ConstString members contain a pointer which isn't 
> aligned so we can't use any bits from the low end of the pointer. Are there 
> any classes that take advantage of high bits in pointers? Most if not all 
> OS's don't use the entire 64 bit address space... It would be great to get 
> lldb_private::FileSpec down to just 2 pointers again.


Although completely unrelated to this patch, getting FileSpec back down to 2 
pointers is easily done.

There are several ways to do this -- either internal to FileSpec itself or made 
available more generally.  Depends on what you want -- all changes involve 
compromises.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39578: Fix a couple of self-assignments using memcpy.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

ping?


https://reviews.llvm.org/D39578



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

[Lldb-commits] [PATCH] D39578: Fix a couple of self-assignments using memcpy.

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 122730.
hintonda added a comment.

- Address comments.


https://reviews.llvm.org/D39578

Files:
  source/Core/RegisterValue.cpp
  source/Core/Value.cpp


Index: source/Core/Value.cpp
===
--- source/Core/Value.cpp
+++ source/Core/Value.cpp
@@ -142,6 +142,9 @@
 }
 
 size_t Value::AppendDataToHostBuffer(const Value &rhs) {
+  if (this == &rhs)
+return 0;
+
   size_t curr_size = m_data_buffer.GetByteSize();
   Status error;
   switch (rhs.GetValueType()) {
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -539,6 +539,9 @@
 }
 
 bool RegisterValue::CopyValue(const RegisterValue &rhs) {
+  if (this == &rhs)
+return rhs.m_type == eTypeInvalid ? false : true;
+
   m_type = rhs.m_type;
   switch (m_type) {
   case eTypeInvalid:


Index: source/Core/Value.cpp
===
--- source/Core/Value.cpp
+++ source/Core/Value.cpp
@@ -142,6 +142,9 @@
 }
 
 size_t Value::AppendDataToHostBuffer(const Value &rhs) {
+  if (this == &rhs)
+return 0;
+
   size_t curr_size = m_data_buffer.GetByteSize();
   Status error;
   switch (rhs.GetValueType()) {
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -539,6 +539,9 @@
 }
 
 bool RegisterValue::CopyValue(const RegisterValue &rhs) {
+  if (this == &rhs)
+return rhs.m_type == eTypeInvalid ? false : true;
+
   m_type = rhs.m_type;
   switch (m_type) {
   case eTypeInvalid:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43432: [cmake] Fix LLDB_CODESIGN_IDENTITY logic.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

This is causing test failures due debugserver not found.  The fix should be to 
add DEBUGSERVER_PATH to the cache -- it was originally available in the root 
scope.

I hope to have a fix in as soon as I validated the fix locally, but if this is 
causing an issue for anyone, please let me know and I'll revert.


Repository:
  rL LLVM

https://reviews.llvm.org/D43432



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

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D36347#901885, @zturner wrote:

> One possible reason for why this never got any traction is that 
> `lldb-commits` wasn't added as a subscriber.  While it's true that the tagged 
> people should have chimed in, having the whole commits list will get some 
> more visibility.  I never saw this come to my inbox.
>
> I think this would be most suitable in the `lldb/examples` folder.
>
> I can't really review this thoroughly, because it relies on a bash script, 
> and I use Windows where we bash isn't really a thing.  My bash is rusty, but 
> it looks like you're embedding a python script in the bash script?  It might 
> be good if this were just an lldb script command.  Take a look at `command 
> script add` in LLDB, and in the `examples` folder for some examples of 
> existing commands that work this way.  The nice thing about doing it this way 
> is that you could just be inside LLDB and write `(lldb) break-diag 
> -Wcovered-switch`, for example, which would be a much tighter integration 
> with the debugger.


Thanks for taking a look.

I mainly work on *nix systems, hence the initial shell script, but if there's 
an interest, I'll be happy to convert it to a single python script as you 
suggest, and resubmit it as a patch to lldb.

Thanks again...


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Thanks for all the feedback.  I'll report back once I've addressed all your 
suggestions.

Thanks again...


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120308.
hintonda added a comment.

Reimplement at a python module.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,124 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+from subprocess import check_output as qx;
+
+# State
+Breakpoints = []
+target = None
+diagtool = None
+
+class MyParser(argparse.ArgumentParser):
+ def format_help(self):
+ return ''' Commands for operating on clang diagnostic breakpoints
+
+Syntax: clangdiag 
+
+The following subcommands are supported:
+
+  enable  -- Enable clang diagnostic breakpoints.
+  disable -- Disable all clang diagnostic breakpoints. 
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+return parser
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+print('id is None')
+return False
+
+global target
+global diagtool
+global Breakpoints
+
+name = qx([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+Breakpoints.append(bp)
+
+return False
+
+def enable(debugger, args):
+# Always disable existing breakpoints
+disable(debugger)
+
+global target
+global diagtool
+global Breakpoints
+
+target = debugger.GetSelectedTarget()
+exe = target.GetExecutable()
+if not exe.Exists():
+print('Target (%s) not set.' % exe.GetFilename())
+return
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if not os.path.exists(diagtool):
+print('diagtool not found along side %s' % exe)
+return
+
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+Breakpoints.append(bp)
+
+return
+
+def disable(debugger):
+global target
+global diagtool
+global Breakpoints
+# Remove all diag breakpoints.
+for bp in Breakpoints:
+target.BreakpointDelete(bp.GetID())
+target = None
+diagtool = None
+Breakpoints = []
+return
+
+def the_diag_command(debugger, command, result, dict):
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would
+command_args = shlex.split(command)
+parser = create_diag_options()
+try:
+args = parser.parse_args(command_args)
+except:
+return
+
+if args.subcommands == 'enable':
+enable(debugger, args)
+else:
+disable(debugger)
+
+return
+
+def __lldb_init_module(debugger, dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Make the options so we can generate the help text for the new LLDB
+# command line command prior to registering it with LLDB below
+parser = create_diag_options()
+the_diag_command.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f clangdiag.the_diag_command clangdiag')
+print 'The "clangdiag" command has been installed, type "help clangdiag" or "clangdiag --help" for detailed help.'
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Thanks for the quick feedback.  I'll make all you suggested changes, but need 
to think a little more about `diagtool`.




Comment at: utils/clangdiag.py:75-78
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if not os.path.exists(diagtool):
+print('diagtool not found along side %s' % exe)
+return

clayborg wrote:
> Allow "diagtool" to be set from an option maybe? This would require the 
> options to be passed into this function as an argument. If it doesn't get 
> set, then modify the options to contain this default value? Then this error 
> message can just complain about the actual path:
> 
> ```
> print('diagtool "%s" doesn't exist' % diagtool)
> ```
The problem is that `diagtool` contains a map of DiagID -> DiagEnums, and must 
be in sync.   I wasn't sure how else to enforce the version you were using was 
built at the same time as the code you're trying to debug.

I can add the option, but then you might get bogus output.


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120340.
hintonda edited the summary of this revision.
hintonda added a comment.
Herald added a subscriber: ilya-biryukov.

- Addressed comments.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,116 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+ def format_help(self):
+ return ''' Commands for operating on clang diagnostic breakpoints
+
+Syntax: clangdiag 
+
+The following subcommands are supported:
+
+  enable  -- Enable clang diagnostic breakpoints.
+  disable -- Disable all clang diagnostic breakpoints. 
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+return parser
+
+def getDiagtool(target):
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+return
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if not os.path.exists(diagtool):
+print('clangdiag: diagtool not found along side %s' % exe)
+return
+return diagtool
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = exe_ctx.GetTarget()
+# Remove all diag breakpoints.
+bkpts = lldb.SBBreakpointList(target)
+target.FindBreakpointsByName("clang::Diagnostic", bkpts)
+for i in range(bkpts.GetSize()):
+target.BreakpointDelete(bkpts.GetBreakpointAtIndex(i).GetID())
+
+return
+
+def the_diag_command(debugger, command, exe_ctx, result, dict):
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would
+command_args = shlex.split(command)
+parser = create_diag_options()
+try:
+args = parser.parse_args(command_args)
+except:
+return
+
+if args.subcommands == 'enable':
+enable(exe_ctx, args)
+else:
+disable(exe_ctx)
+
+return
+
+def __lldb_init_module(debugger, dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Make the options so we can generate the help text for the new LLDB
+# command line command prior to registering it with LLDB below
+parser = create_diag_options()
+the_diag_command.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f clangdiag.the_diag_command clangdiag')
+print 'The "clangdiag" command has been installed, type "help clangdiag" or "clangdiag --help" for detailed help.'
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added inline comments.



Comment at: utils/clangdiag.py:83
+# Remove all diag breakpoints.
+bkpts = lldb.SBBreakpointList(target)
+target.FindBreakpointsByName("clang::Diagnostic", bkpts)

Can't use iterator because it gets invalidated and not all breakpoints get 
removed.  Also, `target.BreakpointDeleteByName` doesn't seem to exist, so 
iterated over `SBBreakpointList instead`.


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120360.
hintonda added a comment.

- Add diagtool option used to set arbitrary diagtool path.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,131 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for operating on clang diagnostic breakpoints
+
+Syntax: clangdiag enable
+clangdiag disable
+clangdiag diagtool 
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Set diagtool path if not in target directory.
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs=1)
+return parser
+
+def getDiagtool(target, diagtool = None):
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = None
+if diagtool:
+getDiagtool.diagtool = diagtool
+elif getDiagtool.diagtool is None:
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+return
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+return
+
+return getDiagtool.diagtool
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = exe_ctx.GetTarget()
+# Remove all diag breakpoints.
+bkpts = lldb.SBBreakpointList(target)
+target.FindBreakpointsByName("clang::Diagnostic", bkpts)
+for i in range(bkpts.GetSize()):
+target.BreakpointDelete(bkpts.GetBreakpointAtIndex(i).GetID())
+
+return
+
+def the_diag_command(debugger, command, exe_ctx, result, dict):
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would
+command_args = shlex.split(command)
+parser = create_diag_options()
+try:
+args = parser.parse_args(command_args)
+except:
+return
+
+if args.subcommands == 'enable':
+enable(exe_ctx, args)
+elif args.subcommands == 'disable':
+disable(exe_ctx)
+else:
+getDiagtool(exe_ctx.GetTarget(), args.path[0])
+
+return
+
+def __lldb_init_module(debugger, dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Make the options so we can generate the help text for the new LLDB
+# command line command prior to registering it with LLDB below
+parser = create_diag_options()
+the_diag_command.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f clangdiag.the_diag_command clangdiag')
+print 'The "clangdiag" command has been installed, type "help clangdiag" or "clangdiag --help" for detailed help.'

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Thanks for the feedback (addressed below).

btw, where should this module go?

Since it's intended for clang, and clang based tool, developers, I put it in 
`clang/utils`, but Zackery suggested `lldb/examples/python` might be a better 
place.  Please let me know if anyone has a preference.




Comment at: utils/clangdiag.py:66
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:

clayborg wrote:
> Is there only ever one of these? If so this is good. 
Currently, DiagnosticsEngine::Report only has two signatures, and both contain 
an `unsigned DiagID` parameter, so this is just a sanity check to make sure we 
are in the right place.




Comment at: utils/clangdiag.py:117
+disable(exe_ctx)
+else:
+getDiagtool(exe_ctx.GetTarget(), args.path[0])

clayborg wrote:
> should probably verify that this 'diagtool' with:
> 
> ```
> elif args.subcommands == 'diagtool':
> ```
> and add an else that creates an error:
> ```
> else:
> result.SetError("invalid subcommand '%s'" % (args.subcommands))
> ```
This is already handled by `argparser`, which will raise an exception if the 
first parameter isn't one of (enable, disable, daigtool).  That's the benefit 
of using `subparsers`.  So, by the time you get to this point, it must be 
"diagtool".

However, I think I should probably verify the path past actually exists.  Plus, 
I think I could add a better exception to alert the user how to fix the problem 
if diagtool couldn't be found in the callback.  Suggestions welcome.


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120459.
hintonda added a comment.

- Enhance diagtool option to check, reset, print out current value.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,137 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for operating on clang diagnostic breakpoints
+
+Syntax: clangdiag enable
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = None
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if getDiagtool.diagtool is None:
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = exe_ctx.GetTarget()
+# Remove all diag breakpoints.
+bkpts = lldb.SBBreakpointList(target)
+target.FindBreakpointsByName("clang::Diagnostic", bkpts)
+for i in range(bkpts.GetSize()):
+target.BreakpointDelete(bkpts.GetBreakpointAtIndex(i).GetID())
+
+return
+
+def the_diag_command(debugger, command, exe_ctx, result, dict):
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would
+command_args = shlex.split(command)
+parser = create_diag_options()
+try:
+args = parser.parse_args(command_args)
+except:
+return
+
+if args.subcommands == 'enable':
+enable(exe_ctx, args)
+elif args.subcommands == 'disable':
+disable(exe_ctx)
+else:
+print(args)
+diagtool = getDiagtool(exe_ctx.GetTarget(), args.path)
+print('diagtool = %s' % diagtool)
+
+return
+
+def __lldb_init_module(debugger, dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Make the options so we can generate the help text for the new LLDB
+# command line command prior to registering it with LLDB below
+parser = create_diag_options()
+the_diag_command.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLD

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D36347#908186, @zturner wrote:

> Do I understand correctly that this will insert breakpoints on *all* clang 
> diagnostics?  That's not necessarily bad, but I was under the impression 
> originally that it would let you pick the diagnostics you wanted to insert 
> breakpoints on.


`clangdiag enable` sets a breakpoint in `DiagnosticsEngine::Report` and adds a 
callback.  The callback passed the value of DiagID to diagtool to get the enum 
spelling of the DiagID, e.g., `err_undeclared_var_use`, then calls 
`BreakpointCreateBySourceRegex` with that string and and empty `FileSpec` to 
set breakpoints in every location that string is seen -- specifically, we use 
"diag::err_undeclared_var_use` for this case.  Now, that might add a few more 
breakpoints than actually needed, but not many, and only rarely in places that 
were actually covered by this particular run.

Also, since we add the breakpoint after it was seen -- 
`DiagnosticsEngine::Report` is called later, and sometimes much later, you'll 
need to run the debugger again to actually hit all the breakpoints.

> Also, What is the workflow for using the "clangdiag diagtool" subcommand?  
> Would you have to do two steps, `clangdiag enable` and then `clangdiag 
> diagtool`?  If so, maybe it could just be `clangdiag enable --diagtool=`

I put `command script import 
/Users/dhinton/projects/llvm_project/llvm/tools/clang/utils/clangdiag.py` in my 
`~/.lldbinit` file, so here's an example of how I use it:

  $ lldb bin/clang-6.0
  The "clangdiag" command has been installed, type "help clangdiag" or 
"clangdiag --help" for detailed help.
  (lldb) target create "bin/clang-6.0"
  Current executable set to 'bin/clang-6.0' (x86_64).
  
  (lldb) clangdiag diagtool
  diagtool = /Users/dhinton/projects/llvm_project/build/Debug/bin/diagtool
  
  (lldb) clangdiag diagtool /bad/path/xx
  clangdiag: /bad/path/xx not found.
  diagtool = /Users/dhinton/projects/llvm_project/build/Debug/bin/diagtool
  
  (lldb) clangdiag diagtool 
/Users/dhinton/projects/monorepo/build/Debug/bin/diagtool
  diagtool = /Users/dhinton/projects/monorepo/build/Debug/bin/diagtool
  
  (lldb) clangdiag diagtool reset
  diagtool = /Users/dhinton/projects/llvm_project/build/Debug/bin/diagtool
  
  (lldb) clangdiag enable
  (lldb) br l
  Current breakpoints:
  1: name = 'DiagnosticsEngine::Report', locations = 33
  Breakpoint commands (Python):
return clangdiag.setDiagBreakpoint(frame, bp_loc, internal_dict)
  
Names:
  clang::Diagnostic
  
1.1: where = clang-6.0`clang::DiagnosticsEngine::Report(unsigned int) + 38 
at Diagnostic.h:1215, address = clang-6.0[0x000100022cd6], unresolved, hit 
count = 0
1.2: where = 
clang-6.0`clang::DiagnosticsEngine::Report(clang::SourceLocation, unsigned int) 
+ 46 at Diagnostic.h:1207, address = clang-6.0[0x00010002c6ce], unresolved, 
hit count = 0
  <...snip...>
  
  (lldb) clangdiag disable
  (lldb) br l
  No breakpoints currently set.
  
  (lldb) clangdiag enable
  (lldb) br l
  Current breakpoints:
  2: name = 'DiagnosticsEngine::Report', locations = 33
  Breakpoint commands (Python):
return clangdiag.setDiagBreakpoint(frame, bp_loc, internal_dict)
  
Names:
  clang::Diagnostic
  
2.1: where = clang-6.0`clang::DiagnosticsEngine::Report(unsigned int) + 38 
at Diagnostic.h:1215, address = clang-6.0[0x000100022cd6], unresolved, hit 
count = 0
2.2: where = 
clang-6.0`clang::DiagnosticsEngine::Report(clang::SourceLocation, unsigned int) 
+ 46 at Diagnostic.h:1207, address = clang-6.0[0x00010002c6ce], unresolved, 
hit count = 0
  <...snip...>
  
  (lldb) run <...>
   might hit one of the new breakpoints if they are seen more than once
  (lldb) run 
   should hit all the breakpoints for which diagnostics were produced
  
  (lldb) br l
  Current breakpoints:
  2: name = 'DiagnosticsEngine::Report', locations = 33, resolved = 33, hit 
count = 5
  Breakpoint commands (Python):
return clangdiag.setDiagBreakpoint(frame, bp_loc, internal_dict)
  
Names:
  clang::Diagnostic
2.1: where = clang-6.0`clang::DiagnosticsEngine::Report(unsigned int) + 38 
at Diagnostic.h:1215, address = 0x000100022cd6, resolved, hit count = 0 

   
2.2: where = 
clang-6.0`clang::DiagnosticsEngine::Report(clang::SourceLocation, unsigned int) 
+ 46 at Diagnostic.h:1207, address = 0x00010002c6ce, resolved, hit count = 0
  <...snip...>
  
  3: source regex = "err_unknown_typename", exact_match = 0, locations = 6, 
resolved = 6, hit count = 2 

  Names:

   

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120492.
hintonda added a comment.

- Remove debugging print statement, and enhance help message.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,161 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Make it always available by adding this to your ~.lldbinit file:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = None
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if getDiagtool.diagtool is None:
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = exe_ctx.GetTarget()
+# Remove all diag breakpoints.
+bkpts = lldb.SBBreakpointList(target)
+target.Fi

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Is there a way to associate a particular diagtool variable to an exe_ctx?


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120518.
hintonda added a comment.

- Maintain process id map for diagtool.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,163 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Make it always available by adding this to your ~.lldbinit file:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+id = target.GetProcess().GetProcessID()
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = {}
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool[id] = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if not id in getDiagtool.diagtool or not getDiagtool.diagtool[id]:
+getDiagtool.diagtool[id] = None
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool[id]
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = e

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120538.
hintonda added a comment.

- Add ability to add breakpoints matching -W warnings.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,175 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import re
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable []
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+
+- If  is passed, just enable the DiagID for that warning.
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Make it always available by adding this to your ~.lldbinit file:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+enable_parser.add_argument('id', nargs='?')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+id = target.GetProcess().GetProcessID()
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = {}
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool[id] = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if not id in getDiagtool.diagtool or not getDiagtool.diagtool[id]:
+getDiagtool.diagtool[id] = None
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool[id]
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+print('clangdiag: id is None')
+return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+
+if args.id:
+diagtool = getDiagtool(target)
+list = subprocess.check_output([diagtool, "list-warnings"]).rstrip();
+for line in list.splitlines(True):
+m = re.

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120628.
hintonda added a comment.

- Add support for individual DiagID's, and print out number of breakpoints 
added.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,192 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import re
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable [|]
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+- Substrings can be passed for both  and .
+- If  is passed, only enable the DiagID(s) for that warning.
+- If  is passed, only enable that DiagID.
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Adding this to your ~.lldbinit file makes clangdiag available at startup:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+enable_parser.add_argument('id', nargs='?')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+id = target.GetProcess().GetProcessID()
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = {}
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool[id] = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if not id in getDiagtool.diagtool or not getDiagtool.diagtool[id]:
+getDiagtool.diagtool[id] = None
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool[id]
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+print('clangdiag: id is None')
+return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+# Make sure we only consider errors, warnings, and extentions.
+# FIXME: Make this configurable?
+prefixes = ['err_', 'warn_', 'exp_']
+if len([prefix for prefix in prefixes+[''] if name.startswith(prefix)][0]):
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Dia

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120629.
hintonda added a comment.

- Remove whitespace.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,192 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import re
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable [|]
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+- Substrings can be passed for both  and .
+- If  is passed, only enable the DiagID(s) for that warning.
+- If  is passed, only enable that DiagID.
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Adding this to your ~.lldbinit file makes clangdiag available at startup:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+enable_parser.add_argument('id', nargs='?')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+id = target.GetProcess().GetProcessID()
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = {}
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool[id] = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if not id in getDiagtool.diagtool or not getDiagtool.diagtool[id]:
+getDiagtool.diagtool[id] = None
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool[id]
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+print('clangdiag: id is None')
+return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+# Make sure we only consider errors, warnings, and extentions.
+# FIXME: Make this configurable?
+prefixes = ['err_', 'warn_', 'exp_']
+if len([prefix for prefix in prefixes+[''] if name.startswith(prefix)][0]):
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+ 

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#911233, @zturner wrote:

> Would you mind deleting and re-creating this revision with lldb-commits added 
> as a subscriber?  I don't think it's sufficient to just "add" it as a 
> subscriber after the fact, I think it has to be done as part of the initial 
> creation, or for some reason it won't show up on the mailing list.


Added lldb-commits to subscribers via arc, so I'm not sure why the initial 
email didn't go out -- always worked with (cfe|llvm)-commits.
But at least it's working now...


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#911237, @zturner wrote:

> Hmm, weird.  Maybe it already is?  Even though I didn't see it on the 
> original email.  Anyway, ignore my last suggestion.
>
> On to actual comments: I'm not really crazy about the `regex:` prefix.  Seems 
> ugly and non-intuitive.  Couldn't we just treat every filename as an argument 
> to `fnmatch (3)`?  Then you don't need any weird prefixes.  Besides, 
> `regex:foo` is a valid filename so if we're going to be making it so that 
> some arguments to `-f` all of a sudden have to be escaped because they're no 
> longer interpreted literally, then we might as well do it in a way that's 
> intuitive and people are already familiar with.


The problem with -f and -s is that they create FileSpec's, and FileSpec has 
some special handling for paths, so I'm not sure how to handle this without a 
prefix.  Is there a delimiter that's invalid on all platforms?  I thought ':' 
would work since we use it for path delimiters, but I've only got a Mac to test 
on right now.  I suppose we could get really ugly and use a uri style prefix, 
e.g., "regex://".

In any case, I think it's worth exploring a solution that doesn't require a lot 
of api changes.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#911263, @zturner wrote:

> In https://reviews.llvm.org/D39436#911254, @hintonda wrote:
>
> > In https://reviews.llvm.org/D39436#911237, @zturner wrote:
> >
> > > Hmm, weird.  Maybe it already is?  Even though I didn't see it on the 
> > > original email.  Anyway, ignore my last suggestion.
> > >
> > > On to actual comments: I'm not really crazy about the `regex:` prefix.  
> > > Seems ugly and non-intuitive.  Couldn't we just treat every filename as 
> > > an argument to `fnmatch (3)`?  Then you don't need any weird prefixes.  
> > > Besides, `regex:foo` is a valid filename so if we're going to be making 
> > > it so that some arguments to `-f` all of a sudden have to be escaped 
> > > because they're no longer interpreted literally, then we might as well do 
> > > it in a way that's intuitive and people are already familiar with.
> >
> >
> > The problem with -f and -s is that they create FileSpec's, and FileSpec has 
> > some special handling for paths, so I'm not sure how to handle this without 
> > a prefix.  Is there a delimiter that's invalid on all platforms?  I thought 
> > ':' would work since we use it for path delimiters, but I've only got a Mac 
> > to test on right now.  I suppose we could get really ugly and use a uri 
> > style prefix, e.g., "regex://".
>
>
> Couldn't they create a *list* of FileSpecs?  Just glancing at the code for 
> `-f`, it does this:
>
>   case 'f':
> m_filenames.AppendIfUnique(FileSpec(option_arg, false));
> break;
>   
>
> So, just resolve the expression at this point and append more than one 
> `FileSpec`?


Yes, passing multiple '-f'  and/or '-s' options is supports, but tedious.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#911260, @jingham wrote:

> I don't like this change.
>
> First off, the whole point of having options in the commands is so that we 
> don't have to have magic encodings in the values.
>
> We also don't have FileSpec's that resolve to multiple files.  What do 
> "Exists", "IsExecutable",  "GetPath" etc. mean in this context?  There would 
> have to be a really strong reason for making this kind of change.  Just 
> avoiding an extra option does not warrant this change so far as I can see.


It seems that FileSpec already has a dual role, and can be either a search 
criteria (possibly representing multiple files) or a resolved file.

The former is used with the '-f' and '-s' parameters, and can easily match 
multiple files, e.g.: if I pass '-f "foo.cpp"', with no directory, it'll match 
every foo.cpp file included in the target or loaded module.  I'd need to check, 
but I think 'Exists', 'IsExecutable', and 'GetPath' would still work as 
expected.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#911264, @jingham wrote:

> Zachary's suggestion is better than adding regex patterns to FileSpec, but I 
> still don't like the idea of encoding option types in the option values.


Are you talking about fnmatch?  Is that portable?  If not, i can rewrite it,.

> Moreover, this doesn't really apply to the main use of -f - file & line 
> breakpoints.  The only plausible use for a source file filter for file & line 
> breakpoints is when you want to restrict breaking on inlined code to a subset 
> of the files that inline it.  But in that case you would want a separate 
> option, since you need to specify the inlined file = with the -f option - and 
> the inlining files - with some other option.
> 
> Wouldn't it be better to add a --source-file-regex option, pick some free 
> single letter, and use that if you want to provide a pattern for the "-p" 
> breakpoint option.  Then we could also use it when specifying some other file 
> filter.

This is valid for all breakpoints, not just those created via the '-p' option.  
I can gen up patch that add `--source-file-regex` to all breakpoints, but I 
also want to use it from python, so that api change might be problematic -- can 
we add additional default parameters without breaking ABI?

Btw, here's an example of how I want to use it -- cuts time to create 
breakpoint by over half, from 15 seconds down to 7 -- which makes my clangdiag 
module load much faster:

  br s -n "DiagnosticsEngine::Report" -f "regex:.*/clang/.*"


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#911274, @jingham wrote:

> BTW, to Z's comment: you can't really resolve the regex pattern when you make 
> the breakpoint.  What if another library gets loaded later on, which has 
> source files that match the source file pattern the user entered, and have 
> source code that matches the -p pattern.  The breakpoint should be updated to 
> encompass those new locations, but you can't do that if you've already 
> matched the pattern against the original list of files, and then thrown away 
> the pattern.


Isn't this how it already works with all breakpoints?  If you load more 
modules, do more locations get automatically loaded?  Sorry if that's a dumb 
question.

> This needs to be maintained as a pattern in the breakpoint, if you want it to 
> be an actual pattern.  But we shouldn't overload FileSpec for that purpose, 
> IMO.

It already is a pattern if used without a path.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#911296, @jingham wrote:

> In https://reviews.llvm.org/D39436#911293, @hintonda wrote:
>
> > In https://reviews.llvm.org/D39436#911260, @jingham wrote:
> >
> > > I don't like this change.
> > >
> > > First off, the whole point of having options in the commands is so that 
> > > we don't have to have magic encodings in the values.
> > >
> > > We also don't have FileSpec's that resolve to multiple files.  What do 
> > > "Exists", "IsExecutable",  "GetPath" etc. mean in this context?  There 
> > > would have to be a really strong reason for making this kind of change.  
> > > Just avoiding an extra option does not warrant this change so far as I 
> > > can see.
> >
> >
> > It seems that FileSpec already has a dual role, and can be either a search 
> > criteria (possibly representing multiple files) or a resolved file.
> >
> > The former is used with the '-f' and '-s' parameters, and can easily match 
> > multiple files, e.g.: if I pass '-f "foo.cpp"', with no directory, it'll 
> > match every foo.cpp file included in the target or loaded module.  I'd need 
> > to check, but I think 'Exists', 'IsExecutable', and 'GetPath' would still 
> > work as expected.
>
>
> There's a big difference between "dir not specified" and a full on glob or 
> regex matching.


Yes, which is what I'm trying to add.  Granted, this might not be acceptable, 
but the searchfilters already work this way.  Not sure how to enhance them to 
support this if we don't use FileSpec -- it's copy constructed in vector, so 
even thought it's passed around as a ref, we can't customize it -- at least 
without changing the vectors to take shared pointers.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoints.

[Don Hinton via Phabricator via lldb-commits]

hintonda created this revision.

Add regex support to file and module names when setting breakpoints.
This solution is enabled via a "regex:" prefix, and doesn't require any other
api changes.

In this example, setting breakpoints for a particular function was twice as
fast using "regex:.*/clang/.*":

  br s -n "DiagnosticsEngine::Report" -f "regex:.*/clang/.*"


https://reviews.llvm.org/D39436

Files:
  include/lldb/Utility/FileSpec.h
  source/Utility/FileSpec.cpp


Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -185,7 +185,8 @@
 //--
 FileSpec::FileSpec(const FileSpec &rhs)
 : m_directory(rhs.m_directory), m_filename(rhs.m_filename),
-  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax) {}
+  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax),
+  m_regex(rhs.m_regex) {}
 
 //--
 // Copy constructor
@@ -209,6 +210,7 @@
 m_filename = rhs.m_filename;
 m_is_resolved = rhs.m_is_resolved;
 m_syntax = rhs.m_syntax;
+m_regex = rhs.m_regex;
   }
   return *this;
 }
@@ -232,6 +234,12 @@
   if (pathname.empty())
 return;
 
+  if(pathname.startswith("regex:")) {
+m_filename.SetString(pathname);
+m_regex.Compile(pathname.substr(6));
+return;
+  }
+
   llvm::SmallString<64> resolved(pathname);
 
   if (resolve) {
@@ -301,6 +309,9 @@
 // Equal to operator
 //--
 bool FileSpec::operator==(const FileSpec &rhs) const {
+  if (m_regex.IsValid())
+return m_regex.Execute(rhs.GetPath());
+
   if (!FileEquals(rhs))
 return false;
   if (DirectoryEquals(rhs))
@@ -411,6 +422,9 @@
 
 bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
  bool remove_backups) {
+  if (a.m_regex.IsValid())
+return a.m_regex.Execute(b.GetPath());
+
   static ConstString g_dot_string(".");
   static ConstString g_dot_dot_string("..");
 
Index: include/lldb/Utility/FileSpec.h
===
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -18,6 +18,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/RegularExpression.h"
 
 #include "llvm/ADT/StringRef.h" // for StringRef
 #include "llvm/Support/FileSystem.h"
@@ -583,6 +584,7 @@
   mutable bool m_is_resolved = false; ///< True if this path has been resolved.
   PathSyntax
   m_syntax; ///< The syntax that this path uses (e.g. Windows / Posix)
+  RegularExpression m_regex;  ///< Regular expression in "regex:" 
prefix passed.
 };
 
 //--


Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -185,7 +185,8 @@
 //--
 FileSpec::FileSpec(const FileSpec &rhs)
 : m_directory(rhs.m_directory), m_filename(rhs.m_filename),
-  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax) {}
+  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax),
+  m_regex(rhs.m_regex) {}
 
 //--
 // Copy constructor
@@ -209,6 +210,7 @@
 m_filename = rhs.m_filename;
 m_is_resolved = rhs.m_is_resolved;
 m_syntax = rhs.m_syntax;
+m_regex = rhs.m_regex;
   }
   return *this;
 }
@@ -232,6 +234,12 @@
   if (pathname.empty())
 return;
 
+  if(pathname.startswith("regex:")) {
+m_filename.SetString(pathname);
+m_regex.Compile(pathname.substr(6));
+return;
+  }
+
   llvm::SmallString<64> resolved(pathname);
 
   if (resolve) {
@@ -301,6 +309,9 @@
 // Equal to operator
 //--
 bool FileSpec::operator==(const FileSpec &rhs) const {
+  if (m_regex.IsValid())
+return m_regex.Execute(rhs.GetPath());
+
   if (!FileEquals(rhs))
 return false;
   if (DirectoryEquals(rhs))
@@ -411,6 +422,9 @@
 
 bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
  bool remove_backups) {
+  if (a.m_regex.IsValid())
+return a.m_regex.Execute(b.GetPath());
+
   static ConstString g_dot_string(".");
   static ConstString g_dot_dot_string("..");
 
Index: include/lldb/Utility/FileSpec.h
===
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -18,6 +18,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/RegularExpression.h"
 
 #include "llvm/ADT/StringRef.h" 

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120933.
hintonda added a comment.

- Remove prefix and add options.


https://reviews.llvm.org/D39436

Files:
  include/lldb/Utility/FileSpec.h
  source/Commands/CommandObjectBreakpoint.cpp
  source/Utility/FileSpec.cpp

Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -180,12 +180,19 @@
 : FileSpec{path, resolve_path,
Triple.isOSWindows() ? ePathSyntaxWindows : ePathSyntaxPosix} {}
 
+FileSpec::FileSpec(llvm::StringRef regex) {
+  m_syntax = ePathSyntaxHostNative;
+  m_filename.SetString(regex);
+  m_regex.Compile(regex);
+}
+
 //--
 // Copy constructor
 //--
 FileSpec::FileSpec(const FileSpec &rhs)
 : m_directory(rhs.m_directory), m_filename(rhs.m_filename),
-  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax) {}
+  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax),
+  m_regex(rhs.m_regex) {}
 
 //--
 // Copy constructor
@@ -209,6 +216,7 @@
 m_filename = rhs.m_filename;
 m_is_resolved = rhs.m_is_resolved;
 m_syntax = rhs.m_syntax;
+m_regex = rhs.m_regex;
   }
   return *this;
 }
@@ -301,6 +309,9 @@
 // Equal to operator
 //--
 bool FileSpec::operator==(const FileSpec &rhs) const {
+  if (m_regex.IsValid())
+return m_regex.Execute(rhs.GetPath());
+
   if (!FileEquals(rhs))
 return false;
   if (DirectoryEquals(rhs))
@@ -411,6 +422,9 @@
 
 bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
  bool remove_backups) {
+  if (a.m_regex.IsValid())
+return a.m_regex.Execute(b.GetPath());
+
   static ConstString g_dot_string(".");
   static ConstString g_dot_dot_string("..");
 
Index: source/Commands/CommandObjectBreakpoint.cpp
===
--- source/Commands/CommandObjectBreakpoint.cpp
+++ source/Commands/CommandObjectBreakpoint.cpp
@@ -256,6 +256,8 @@
   { LLDB_OPT_NOT_10,   false, "shlib",  's', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eModuleCompletion, eArgTypeShlibName,   "Set the breakpoint only in this shared library.  Can repeat this option "
   "multiple times to specify multiple shared libraries." },
   { LLDB_OPT_SET_ALL,  false, "hardware",   'H', OptionParser::eNoArgument,   nullptr, nullptr, 0, eArgTypeNone,"Require the breakpoint to use hardware breakpoints." },
+  { LLDB_OPT_FILE, false, "source-file-regex",  'z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression,   "Only files matching pattern." },
+  { LLDB_OPT_FILE, false, "module-regex",   'Z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression,   "Only modules matching pattern." },
   { LLDB_OPT_FILE, false, "file",   'f', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeFilename,"Specifies the source file in which to set this breakpoint.  Note, by default "
   "lldb only looks for files that are #included if they use the standard include "
   "file extensions.  To set breakpoints on .c/.cpp/.m/.mm files that are "
@@ -560,6 +562,14 @@
 m_source_regex_func_names.insert(option_arg);
 break;
 
+  case 'z':
+m_filenames.AppendIfUnique(FileSpec(option_arg));
+break;
+
+  case 'Z':
+m_modules.AppendIfUnique(FileSpec(option_arg));
+break;
+
   default:
 error.SetErrorStringWithFormat("unrecognized option '%c'",
short_option);
Index: include/lldb/Utility/FileSpec.h
===
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -18,6 +18,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/RegularExpression.h"
 
 #include "llvm/ADT/StringRef.h" // for StringRef
 #include "llvm/Support/FileSystem.h"
@@ -92,6 +93,8 @@
   explicit FileSpec(llvm::StringRef path, bool resolve_path,
 const llvm::Triple &Triple);
 
+  explicit FileSpec(llvm::StringRef regex);
+
   //--
   /// Copy constructor
   ///
@@ -583,6 +586,7 @@
   mutable bool m_is_resolved = false; ///< True if this path has been resolved.
   PathSynt

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.



In https://reviews.llvm.org/D39436#911875, @clayborg wrote:

> A few general things: don't modify FileSpec, we have many of these objects 
> and we can't increase their size without directly affecting memory usage. 
> FileSpec objects represent one file on disk, not multiple. We should make a 
> new class that contains a FileSpec and a regex, but not put it into FileSpec.


Will do...

Btw, if size is an issue, would it make sense to add a type to the enum 
declarations?  e.g.:

  diff --git a/include/lldb/Utility/FileSpec.h b/include/lldb/Utility/FileSpec.h
  index 67926d01e..55d44d840 100644
  --- a/include/lldb/Utility/FileSpec.h
  +++ b/include/lldb/Utility/FileSpec.h
  @@ -61,7 +61,7 @@ namespace lldb_private {
   //--
   class FileSpec {
   public:
  -  enum PathSyntax {
  +  enum PathSyntax : unsigned char {
   ePathSyntaxPosix,
   ePathSyntaxWindows,
   ePathSyntaxHostNative


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Actually, it wouldn't matter for this one due to alignment.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#912829, @clayborg wrote:

> In https://reviews.llvm.org/D39436#912828, @zturner wrote:
>
> > In https://reviews.llvm.org/D39436#912810, @clayborg wrote:
> >
> > > I was unhappy when we went over two pointers for a FileSpec when m_syntax 
> > > was added due to the extra size. Anything we can do to make this smaller 
> > > would be great, so the type on the enum would work, but as you say the 
> > > alignment will nullify that. The two ConstString members contain a 
> > > pointer which isn't aligned so we can't use any bits from the low end of 
> > > the pointer. Are there any classes that take advantage of high bits in 
> > > pointers? Most if not all OS's don't use the entire 64 bit address 
> > > space... It would be great to get lldb_private::FileSpec down to just 2 
> > > pointers again.
> >
> >
> > `ConstString` doesn't *currently* contain aligned pointers, but there's no 
> > reason we couldn't make it contain aligned pointers.  Then we could use 
> > `llvm::PointerUnion`.
>
>
> I would be fine with that.
>
> > That said, I want to state again that I think this change is the wrong 
> > direction.  I don't think we need this functionality in `FileSpec`, or even 
> > in another class.  I think it is better served in the script.
>
> Agreed as well. I do like the idea of source path regexes, but only if we 
> have a real need to add it to the API.


I haven't had time to really look into this, but it seems that maintaining two 
independent strings, one for directory and one for basename, is just for 
convenience.   We could easily keep it in a single string with an index to 
basename.  When the directory portion is updated, e.g., resolved, etc., we just 
overwrite the string and adjust the index.  And adjust accessors as needed.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D39436#912910, @zturner wrote:

>




>> I haven't had time to really look into this, but it seems that maintaining 
>> two independent strings, one for directory and one for basename, is just for 
>> convenience.   We could easily keep it in a single string with an index to 
>> basename.  When the directory portion is updated, e.g., resolved, etc., we 
>> just overwrite the string and adjust the index.  And adjust accessors as 
>> needed.
> 
> The reason it's two strings is for memory efficiency and de-duplication.  
> Suppose you make `FileSpec` instances from  `foo/bar/baz/` and 
> `foo/bar/buzz`.  This gets separated into 4 instances of `ConstString`.  
> `foo/bar`, `baz`, `foo/bar`, and `buzz`.  Because `ConstString`s are pooled, 
> there's actually only 3 strings here.  `foo/bar`, `baz`, and `buzz`.
> 
> It probably doesn't seem like a lot, but over the course of thousands and 
> thousands of files (which debuggers often examine and which often share a 
> common parent directory) this is a large memory savings.

Ah, thanks for the explanation.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 121214.
hintonda added a comment.

Addressed @clayborg's comments.

- Added FileSpec ctor that takes an explicit RegularExpression object when 
using FileSpec as a regex pattern.

- Removed the RegularExpression member variable, added a flag, and create the 
regex on demand in operator==().

- Added type to PathSyntax enum, and reordered member variables from, largest 
to smallest (evens thought the last three are now the same size) to make sure 
sizeof(FileSpec) doesn't grow with this change.

- Removed implementation of special methods that had default behavior, and 
declared the default ctor '= default', which should reduce the maintenance 
burden.

Next steps:

- Create SBRegularExpression and add ctor to SBFileSpec that takes it as 
parameter.  That will allow this use case without any other changes:

  regex = lldb.SBRegularExpression(".*/some/path/.*") 
target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec(regex))

- Then wire up SBFileSpec to do the right thing.  SBRegularExpression can 
validate the expression on construction to give immediate feedback.


https://reviews.llvm.org/D39436

Files:
  include/lldb/Utility/FileSpec.h
  source/Commands/CommandObjectBreakpoint.cpp
  source/Utility/FileSpec.cpp

Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -164,8 +164,6 @@
   }
 }
 
-FileSpec::FileSpec() : m_syntax(GetNativeSyntax()) {}
-
 //--
 // Default constructor that can take an optional full path to a
 // file on disk.
@@ -180,12 +178,12 @@
 : FileSpec{path, resolve_path,
Triple.isOSWindows() ? ePathSyntaxWindows : ePathSyntaxPosix} {}
 
-//--
-// Copy constructor
-//--
-FileSpec::FileSpec(const FileSpec &rhs)
-: m_directory(rhs.m_directory), m_filename(rhs.m_filename),
-  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax) {}
+FileSpec::FileSpec(RegularExpression regex) {
+  if(regex.IsValid()) {
+m_filename.SetString(regex.GetText());
+m_is_regex = true;
+  }
+}
 
 //--
 // Copy constructor
@@ -196,24 +194,6 @@
 }
 
 //--
-// Virtual destructor in case anyone inherits from this class.
-//--
-FileSpec::~FileSpec() {}
-
-//--
-// Assignment operator.
-//--
-const FileSpec &FileSpec::operator=(const FileSpec &rhs) {
-  if (this != &rhs) {
-m_directory = rhs.m_directory;
-m_filename = rhs.m_filename;
-m_is_resolved = rhs.m_is_resolved;
-m_syntax = rhs.m_syntax;
-  }
-  return *this;
-}
-
-//--
 // Update the contents of this object with a new path. The path will
 // be split up into a directory and filename and stored as uniqued
 // string values for quick comparison and efficient memory usage.
@@ -301,6 +281,12 @@
 // Equal to operator
 //--
 bool FileSpec::operator==(const FileSpec &rhs) const {
+  if(m_is_regex) {
+RegularExpression regex(m_filename.GetStringRef());
+if (regex.IsValid())
+  return regex.Execute(rhs.GetPath());
+  }
+
   if (!FileEquals(rhs))
 return false;
   if (DirectoryEquals(rhs))
@@ -411,6 +397,9 @@
 
 bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
  bool remove_backups) {
+  if (a.m_is_regex)
+return a == b;
+
   static ConstString g_dot_string(".");
   static ConstString g_dot_dot_string("..");
 
Index: source/Commands/CommandObjectBreakpoint.cpp
===
--- source/Commands/CommandObjectBreakpoint.cpp
+++ source/Commands/CommandObjectBreakpoint.cpp
@@ -256,6 +256,8 @@
   { LLDB_OPT_NOT_10,   false, "shlib",  's', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eModuleCompletion, eArgTypeShlibName,   "Set the breakpoint only in this shared library.  Can repeat this option "
   "multiple times to specify multiple shared libraries." },
   { LLDB_OPT_SET_ALL,  false, "hardware",   'H', OptionParser::eNoArgument,   nullptr, nullptr, 0, eArgTypeNone,"Require the breakpoint to use hardware breakpoints." },
+  { LLDB_OPT_FILE, false, "source-file-regex",  'z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompleti

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added inline comments.



Comment at: include/lldb/Utility/FileSpec.h:65-69
+  enum PathSyntax : unsigned char {
 ePathSyntaxPosix,
 ePathSyntaxWindows,
 ePathSyntaxHostNative
   };

zturner wrote:
> This is actually a very nice change, as it reduces the size of `FileSpec` by 
> a couple of bytes.  I think you can submit this change as a one-liner by 
> itself, independent of this patch.
I suppose it depends on you compiler/OS, but this by it self doesn't change the 
size of FileSpec at all -- just changes the padding from 3 to 6.  It's still 
the size of 3 pointers due to alignment -- at least that's my understanding.

However, if you did have a way to encode this stuff into the two existing 
pointers, it might help -- you still need at least 4 bits if I'm not mistaken.



Comment at: include/lldb/Utility/FileSpec.h:557
   mutable bool m_is_resolved = false; ///< True if this path has been resolved.
-  PathSyntax
-  m_syntax; ///< The syntax that this path uses (e.g. Windows / Posix)
+  bool m_is_regex = false;///< Filename is a regular expression.
 };

zturner wrote:
> I thought in previous comments we had decided that this wasn't really the 
> right direction, and that `FileSpec` should represent one file.  //If// we 
> want this functionality in LLDB (and again, I'm not convinced), it should be 
> done in such a way that the `FileSpec` class remains unmodified.  We should 
> not have to touch this class for any of this.
I actually spent quite a bit of time trying to do it the other way, i.e., not 
touching FileSpec, but the diff got so big, I decided it was probably a 
mistake.  I still have it in a local branch, but it's not ready to commit.



https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda abandoned this revision.
hintonda added a comment.

Okay, got the message.

Sorry for the noise.


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda added inline comments.



Comment at: include/lldb/Utility/FileSpec.h:65-69
+  enum PathSyntax : unsigned char {
 ePathSyntaxPosix,
 ePathSyntaxWindows,
 ePathSyntaxHostNative
   };

zturner wrote:
> hintonda wrote:
> > zturner wrote:
> > > This is actually a very nice change, as it reduces the size of `FileSpec` 
> > > by a couple of bytes.  I think you can submit this change as a one-liner 
> > > by itself, independent of this patch.
> > I suppose it depends on you compiler/OS, but this by it self doesn't change 
> > the size of FileSpec at all -- just changes the padding from 3 to 6.  It's 
> > still the size of 3 pointers due to alignment -- at least that's my 
> > understanding.
> > 
> > However, if you did have a way to encode this stuff into the two existing 
> > pointers, it might help -- you still need at least 4 bits if I'm not 
> > mistaken.
> It's possible for `sizeof(int)` to be equal to the size of a pointer.  This 
> happens **always** when building x86, but it can happen on x64 too.  [[ 
> https://godbolt.org/g/GN91oZ | For example ]].  Note that it returns 12.  If 
> you remove the specification of the underlying type, it returns 16 instead.
> 
You're absolutely correct.  I don't do much on 32 bit, but this would 
definitely help when compiling with -m32.

Good catch...


https://reviews.llvm.org/D39436



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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 121350.
hintonda added a comment.

I realize this probably isn't an acceptable change, but it was a good
learing experience for me, as I'd never looked at the lldb code before
last Sunday, so I figured I'd go ahead and complete it.

While the basic technique remains the same, this version just adds a
single enum value, ePathSyntaxRegex, and modifies PathSyntaxIsPosix()
to treat it as a posix path so it doesn't get modified.

The logic for testing the regex was moved to
FileSpecList::FindFileIndex() which does the filtering and already
deals with equality issues, e.g., whether or not to include the
directory in the match.

Finally, the SB API was modified to use accept a new regex flag, and a
test was added.

Thanks again for all your feedback and suggestions...


https://reviews.llvm.org/D39436

Files:
  include/lldb/API/SBFileSpec.h
  include/lldb/Utility/FileSpec.h
  packages/Python/lldbsuite/test/python_api/breakpoint/TestBreakpointAPI.py
  scripts/interface/SBFileSpec.i
  source/API/SBFileSpec.cpp
  source/Commands/CommandObjectBreakpoint.cpp
  source/Core/FileSpecList.cpp
  source/Utility/FileSpec.cpp

Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -48,7 +48,8 @@
 bool PathSyntaxIsPosix(FileSpec::PathSyntax syntax) {
   return (syntax == FileSpec::ePathSyntaxPosix ||
   (syntax == FileSpec::ePathSyntaxHostNative &&
-   GetNativeSyntax() == FileSpec::ePathSyntaxPosix));
+   GetNativeSyntax() == FileSpec::ePathSyntaxPosix) ||
+  syntax == FileSpec::ePathSyntaxRegex);
 }
 
 const char *GetPathSeparators(FileSpec::PathSyntax syntax) {
Index: source/Core/FileSpecList.cpp
===
--- source/Core/FileSpecList.cpp
+++ source/Core/FileSpecList.cpp
@@ -10,6 +10,7 @@
 #include "lldb/Core/FileSpecList.h"
 
 #include "lldb/Utility/ConstString.h" // for ConstString
+#include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
 
 #include  // for find
@@ -90,6 +91,12 @@
   bool compare_filename_only = file_spec.GetDirectory().IsEmpty();
 
   for (size_t idx = start_idx; idx < num_files; ++idx) {
+if (m_files[idx].GetPathSyntax() == FileSpec::ePathSyntaxRegex) {
+  RegularExpression regex(m_files[idx].GetPath(false));
+  if (regex.Execute(file_spec.GetPath()))
+return idx;
+  continue;
+}
 if (compare_filename_only) {
   if (ConstString::Equals(
   m_files[idx].GetFilename(), file_spec.GetFilename(),
Index: source/Commands/CommandObjectBreakpoint.cpp
===
--- source/Commands/CommandObjectBreakpoint.cpp
+++ source/Commands/CommandObjectBreakpoint.cpp
@@ -256,6 +256,8 @@
   { LLDB_OPT_NOT_10,   false, "shlib",  's', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eModuleCompletion, eArgTypeShlibName,   "Set the breakpoint only in this shared library.  Can repeat this option "
   "multiple times to specify multiple shared libraries." },
   { LLDB_OPT_SET_ALL,  false, "hardware",   'H', OptionParser::eNoArgument,   nullptr, nullptr, 0, eArgTypeNone,"Require the breakpoint to use hardware breakpoints." },
+  { LLDB_OPT_FILE, false, "source-file-regex",  'z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression,   "Only files matching pattern." },
+  { LLDB_OPT_FILE, false, "module-regex",   'Z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression,   "Only modules matching pattern." },
   { LLDB_OPT_FILE, false, "file",   'f', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeFilename,"Specifies the source file in which to set this breakpoint.  Note, by default "
   "lldb only looks for files that are #included if they use the standard include "
   "file extensions.  To set breakpoints on .c/.cpp/.m/.mm files that are "
@@ -560,6 +562,16 @@
 m_source_regex_func_names.insert(option_arg);
 break;
 
+  case 'z':
+m_filenames.AppendIfUnique(
+FileSpec(option_arg, false, FileSpec::ePathSyntaxRegex));
+break;
+
+  case 'Z':
+m_modules.AppendIfUnique(
+FileSpec(option_arg, false, FileSpec::ePathSyntaxRegex));
+break;
+
   default:
 error.SetErrorStringWithFormat("unrecognized option '%c'",
short_option);
Index: source/API/SBFileSpec.cpp
===
--- sour

[Lldb-commits] [PATCH] D39574: Add type to FileSpec::PathSyntax enum.

[Don Hinton via Phabricator via lldb-commits]

hintonda created this revision.

Add type to FileSpec::PathSyntax enum to decrease size for
FileSpec on systems with 32 bit pointers.

Thanks to @zturner for pointing this out.


https://reviews.llvm.org/D39574

Files:
  include/lldb/Utility/FileSpec.h


Index: include/lldb/Utility/FileSpec.h
===
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -61,7 +61,7 @@
 //--
 class FileSpec {
 public:
-  enum PathSyntax {
+  enum PathSyntax : unsigned char {
 ePathSyntaxPosix,
 ePathSyntaxWindows,
 ePathSyntaxHostNative


Index: include/lldb/Utility/FileSpec.h
===
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -61,7 +61,7 @@
 //--
 class FileSpec {
 public:
-  enum PathSyntax {
+  enum PathSyntax : unsigned char {
 ePathSyntaxPosix,
 ePathSyntaxWindows,
 ePathSyntaxHostNative
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39578: Fix a couple of self-assignments using memcpy.

[Don Hinton via Phabricator via lldb-commits]

hintonda created this revision.

These two methods are essentially assignents, but don't check
for self-assignment and use memcpy for member variables.

Since they aren't actually operator=(), it's unclear what should be done:

- check and shortcut
- assert
- allow but s/memcpy/memmove/


https://reviews.llvm.org/D39578

Files:
  source/Core/RegisterValue.cpp
  source/Core/Value.cpp


Index: source/Core/Value.cpp
===
--- source/Core/Value.cpp
+++ source/Core/Value.cpp
@@ -143,6 +143,8 @@
 }
 
 size_t Value::AppendDataToHostBuffer(const Value &rhs) {
+  // FIXME: What should we do if this == &rhs?
+  // If we allow, change s/memcpy/memmove/ below.
   size_t curr_size = m_data_buffer.GetByteSize();
   Status error;
   switch (rhs.GetValueType()) {
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -539,6 +539,11 @@
 }
 
 bool RegisterValue::CopyValue(const RegisterValue &rhs) {
+  // Maintain current behavior, but guard against self-assignment (see memcpy
+  // below).?
+  if (this == &rhs)
+return rhs.m_type == eTypeInvalid ? false : true;
+
   m_type = rhs.m_type;
   switch (m_type) {
   case eTypeInvalid:


Index: source/Core/Value.cpp
===
--- source/Core/Value.cpp
+++ source/Core/Value.cpp
@@ -143,6 +143,8 @@
 }
 
 size_t Value::AppendDataToHostBuffer(const Value &rhs) {
+  // FIXME: What should we do if this == &rhs?
+  // If we allow, change s/memcpy/memmove/ below.
   size_t curr_size = m_data_buffer.GetByteSize();
   Status error;
   switch (rhs.GetValueType()) {
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -539,6 +539,11 @@
 }
 
 bool RegisterValue::CopyValue(const RegisterValue &rhs) {
+  // Maintain current behavior, but guard against self-assignment (see memcpy
+  // below).?
+  if (this == &rhs)
+return rhs.m_type == eTypeInvalid ? false : true;
+
   m_type = rhs.m_type;
   switch (m_type) {
   case eTypeInvalid:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39580: Quash a few 'warning: comparison of function 'compression_decode_buffer' not equal to a null pointer is always true [-Wtautologica l-pointer-compare]'

[Don Hinton via Phabricator via lldb-commits]

hintonda created this revision.

Quash several warnings of this type using clang's suggested fix:

[2741/3631] Building CXX object 
tools/lldb/source/Plugins/Process/gdb-remote/CMakeFiles/lldbPluginProcessGDBRemote.dir/GDBRemoteCommunication.cpp.o
/Users/dhinton/projects/llvm_project/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:603:7:
 warning: comparison of function 'compression_decode_buffer' not equal to a 
null pointer is always true [-Wtautological-pointer-compare]

  if (compression_decode_buffer != NULL &&
  ^

/Users/dhinton/projects/llvm_project/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:603:7:
 note: prefix with the address-of operator to silence this warning

  if (compression_decode_buffer != NULL &&
  ^
  &


https://reviews.llvm.org/D39580

Files:
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp


Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1024,7 +1024,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so test if compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   avail_type == CompressionType::None) {
 for (auto compression : supported_compressions) {
   if (compression == "lzfse") {
@@ -1039,7 +1039,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so test if compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   avail_type == CompressionType::None) {
 for (auto compression : supported_compressions) {
   if (compression == "zlib-deflate") {
@@ -1066,7 +1066,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so test if compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   avail_type == CompressionType::None) {
 for (auto compression : supported_compressions) {
   if (compression == "lz4") {
@@ -1081,7 +1081,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so test if compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   avail_type == CompressionType::None) {
 for (auto compression : supported_compressions) {
   if (compression == "lzma") {
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -600,7 +600,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so check that compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   (m_compression_type == CompressionType::ZlibDeflate ||
m_compression_type == CompressionType::LZFSE ||
m_compression_type == CompressionType::LZ4)) {


Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1024,7 +1024,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so test if compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   avail_type == CompressionType::None) {
 for (auto compression : supported_compressions) {
   if (compression == "lzfse") {
@@ -1039,7 +1039,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so test if compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   avail_type == CompressionType::None) {
 for (auto compression : supported_compressions) {
   if (compression == "zlib-deflate") {
@@ -1066,7 +1066,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak linked so test if compression_decode_buffer() is
   // available
-  if (compression_decode_buffer != NULL &&
+  if (&compression_decode_buffer != NULL &&
   avail_type == CompressionType::None) {
 for (auto compression : supported_compressions) {
   if (compression == "lz4") {
@@ -1081,7 +1081,7 @@
 #if defined(HAVE_LIBCOMPRESSION)
   // libcompression is weak l

[Lldb-commits] [PATCH] D70259: [Error] Add source location to cantFail

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 230125.
hintonda added a comment.
Herald added subscribers: lldb-commits, cfe-commits, usaxena95, kadircet, 
arphaman, jkorous.
Herald added projects: clang, LLDB.

- Replace macro magic with matching 3-parameter template functions.
- Refactor cantFail move into detail namespace.  Change macro name to


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70259

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FormatTests.cpp
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/unittests/Basic/DiagnosticTest.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/unittests/Target/ExecutionContextTest.cpp
  llvm/include/llvm/Support/Error.h
  llvm/unittests/Support/ErrorTest.cpp

Index: llvm/unittests/Support/ErrorTest.cpp
===
--- llvm/unittests/Support/ErrorTest.cpp
+++ llvm/unittests/Support/ErrorTest.cpp
@@ -390,8 +390,8 @@
   };
 
   EXPECT_DEATH(FailToHandle(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << "Unhandled Error in handleAllErrors call did not cause an "
  "abort()";
 }
@@ -410,8 +410,8 @@
   };
 
   EXPECT_DEATH(ReturnErrorFromHandler(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << " Error returned from handler in handleAllErrors call did not "
  "cause abort()";
 }
@@ -515,8 +515,8 @@
   EXPECT_DEATH(cantFail(make_error("Original error message",
 inconvertibleErrorCode()),
 "Cantfail call failed"),
-   "Cantfail call failed\n"
-   "Original error message")
+   "Original error message\n"
+   "Cantfail call failed")
   << "cantFail(Error) did not cause an abort for failure value";
 
   EXPECT_DEATH(
@@ -530,6 +530,24 @@
 }
 #endif
 
+TEST(Error, CantFailSourceLocation) {
+  std::string ErrStr;
+  raw_string_ostream OS(ErrStr);
+  OS << "\nFailure value returned from cantFail wrapped call";
+
+#if defined(NDEBUG)
+  // __FILE__ and __LINE_ not added
+  OS << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#else
+  // __FILE__ and __LINE__ added
+  int Line = __LINE__;
+  OS << " at " << __FILE__ << ":" << (Line + 2) << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#endif
+}
 
 // Test Checked Expected in success mode.
 TEST(Error, CheckedExpectedInSuccessMode) {
Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -687,6 +687,23 @@
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
 bool gen_crash_diag = true);
 
+namespace detail {
+
+inline LLVM_ATTRIBUTE_NORETURN void
+handleError(Error Err, const char *Msg, const char *file, unsigned line) {
+  if (!Msg)
+Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+  std::string Str;
+  raw_string_ostream OS(Str);
+  OS << Err << "\n" << Msg;
+  if (file)
+OS << " at " << file << ":" << line;
+  Msg = OS.str().c_str();
+#endif
+  llvm_unreachable(Msg);
+}
+
 /// Report a fatal error if Err is a failure value.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
@@ -700,18 +717,10 @@
 ///
 

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Btw, options on the command line always override what's in the cache.  Has 
nothing to do with FORCE.  All FORCE does is make sure the set command actually 
changes an existing cache value.  So it's an ordering issue.  If the `-D` comes 
before the `-C` then using FORCE would override, but if the `-C` comes before 
the `-D`, the `-D` always overrides.

I always use FORCE and put my overrides after the cache file.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61956



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

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

[Don Hinton via Phabricator via lldb-commits]

hintonda added inline comments.



Comment at: lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt:12
+
+target_sources(LLDBMiUtilTests PRIVATE $)

Just wanted to let you know that using `$` in anything 
other than `add_library` and `add_executable` wasn't added until cmake 3.9, so 
I'm unable to configure lldb using the officially supported cmake 3.4.3 version.

Btw, I use cmake 3.4.3 locally so I don't unintentionally use new unsupported 
features to the clang/llvm cmake files.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D55653



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

[Lldb-commits] [PATCH] D63544: Add a worlaround for unsupported cmake feature

[Don Hinton via Phabricator via lldb-commits]

hintonda added inline comments.



Comment at: tools/lldb/unittests/tools/lldb-mi/utils/CMakeLists.txt:16
+else()
+  target_sources(LLDBMiUtilTests PRIVATE $)
+endif()

I'm assuming from your comment, that this is a performance issue.  If so, could 
you add a comment and also update the commit message so people can appreciate 
the need for the added complexity?

Btw, just tested with 3.4.3, and your change made cmake happy.  Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63544



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

[Lldb-commits] [PATCH] D63544: Add a worlaround for unsupported cmake feature

[Don Hinton via Phabricator via lldb-commits]

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

Otherwise, LGTM, thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63544



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

[Lldb-commits] [PATCH] D63544: Add a worlaround for unsupported cmake feature

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

I just reviewed all the release notes, and while 3.9 relaxed the usage of 
TARGET_OBJECTS, cmake doesn't explicitly mention allowing them in 
`target_link_libraries` until release 3.15.

I don't have any of those versions, but it looks like 3.15 is probably the 
version you need to use instead of 3.9.  Please see: 
https://cmake.org/cmake/help/latest/release/3.15.html


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63544



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

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

[Don Hinton via Phabricator via lldb-commits]

hintonda added inline comments.



Comment at: lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt:12
+
+target_sources(LLDBMiUtilTests PRIVATE $)

tatyana-krasnukha wrote:
> hintonda wrote:
> > Just wanted to let you know that using `$` in anything 
> > other than `add_library` and `add_executable` wasn't added until cmake 3.9, 
> > so I'm unable to configure lldb using the officially supported cmake 3.4.3 
> > version.
> > 
> > Btw, I use cmake 3.4.3 locally so I don't unintentionally use new 
> > unsupported features to the clang/llvm cmake files.
> > 
> Thank you for pointing to this issue. I had cmake 3.6.2 version installed 
> and, oddly enough, it worked well.
> I'd appreciate your review of the [[https://reviews.llvm.org/D63544 | patch 
> ]] that fixes the issue.
I don't have all the various version installed, so I was trying to figure out 
which version by looking at the online docs.  If 3.6.2 works, then it changed 
somewhere between 3.4.3 and 3.6.2, so I guess you can just use >= 3.6.2 for 
your test.

Thanks again for looking into this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55653



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

[Lldb-commits] [PATCH] D63544: Use object library if cmake supports it

[Don Hinton via Phabricator via lldb-commits]

hintonda accepted this revision.
hintonda added a comment.

In D63544#1551939 , @tatyana-krasnukha 
wrote:

> Finally updated versions - `target_sources` supports using 
> `$` since CMake 3.5.0.


Great, thanks for narrowing that down,

Still LGTM!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63544



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

[Lldb-commits] [PATCH] D70259: [Error] Add source location to cantFail

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 230131.
hintonda added a comment.

- Add back original llvm::cantFail signatures so they'll still be


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70259

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FormatTests.cpp
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/unittests/Basic/DiagnosticTest.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/unittests/Target/ExecutionContextTest.cpp
  llvm/include/llvm/Support/Error.h
  llvm/unittests/Support/ErrorTest.cpp

Index: llvm/unittests/Support/ErrorTest.cpp
===
--- llvm/unittests/Support/ErrorTest.cpp
+++ llvm/unittests/Support/ErrorTest.cpp
@@ -390,8 +390,8 @@
   };
 
   EXPECT_DEATH(FailToHandle(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << "Unhandled Error in handleAllErrors call did not cause an "
  "abort()";
 }
@@ -410,8 +410,8 @@
   };
 
   EXPECT_DEATH(ReturnErrorFromHandler(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << " Error returned from handler in handleAllErrors call did not "
  "cause abort()";
 }
@@ -515,8 +515,8 @@
   EXPECT_DEATH(cantFail(make_error("Original error message",
 inconvertibleErrorCode()),
 "Cantfail call failed"),
-   "Cantfail call failed\n"
-   "Original error message")
+   "Original error message\n"
+   "Cantfail call failed")
   << "cantFail(Error) did not cause an abort for failure value";
 
   EXPECT_DEATH(
@@ -530,6 +530,24 @@
 }
 #endif
 
+TEST(Error, CantFailSourceLocation) {
+  std::string ErrStr;
+  raw_string_ostream OS(ErrStr);
+  OS << "\nFailure value returned from cantFail wrapped call";
+
+#if defined(NDEBUG)
+  // __FILE__ and __LINE_ not added
+  OS << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#else
+  // __FILE__ and __LINE__ added
+  int Line = __LINE__;
+  OS << " at " << __FILE__ << ":" << (Line + 2) << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#endif
+}
 
 // Test Checked Expected in success mode.
 TEST(Error, CheckedExpectedInSuccessMode) {
Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -687,6 +687,48 @@
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
 bool gen_crash_diag = true);
 
+namespace detail {
+
+inline LLVM_ATTRIBUTE_NORETURN void handleError(Error Err, const char *Msg,
+const char *file = nullptr,
+unsigned line = 0) {
+  if (!Msg)
+Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+  std::string Str;
+  raw_string_ostream OS(Str);
+  OS << Err << "\n" << Msg;
+  if (file)
+OS << " at " << file << ":" << line;
+  Msg = OS.str().c_str();
+#endif
+  llvm_unreachable(Msg);
+}
+
+inline void cantFail(const char *file, unsigned line, Error Err,
+ const char *Msg = nullptr) {
+  if (Err)
+handleError(std::move(Err), Msg, file, line);
+}
+
+template 
+T cantFail(const char *file, unsigned line, Expected ValOrEr