[Lldb-commits] [PATCH] D39283: [lldb-dev] Update LLDB test cases for 'inlineStepping'

2017-10-25 Thread Carlos Alberto Enciso via Phabricator via lldb-commits
CarlosAlbertoEnciso created this revision.
Herald added a subscriber: eraman.

A patch to correct the compiler issue, where instructions associated to
the function prolog are assigned line information, causing the debugger
to show incorrectly the beginning of the function body, caused some tests
in the LLDB suite to fail.

For a full description, please see:

https://reviews.llvm.org/rL313047
https://reviews.llvm.org/D37625

This patch include the required changes to the failing tests.

For example, using 'caller_trivial_1' from the test case:

  void
  caller_trivial_1 ()
  {
  caller_trivial_2(); // In caller_trivial_1.
  inline_value += 1; 
  }

The "sub $0x8,%esp" instruction, which is frame setup code is printed as
being part of the statement 'inline_value += 1

  void
  caller_trivial_1 ()
  {
c0:55   push   %ebp
c1:89 e5mov%esp,%ebp
  inline_value += 1;  // At first increment in caller_trivial_1.
c3:83 ec 08 sub$0x8,%esp
c6:a1 00 00 00 00   mov0x0,%eax
cb:83 c0 01 add$0x1,%eax
ce:a3 00 00 00 00   mov%eax,0x0
  caller_trivial_2(); // In caller_trivial_1.
d3:e8 18 00 00 00   call   f0 <_Z16caller_trivial_2v>
  inline_value += 1;
d8:a1 00 00 00 00   mov0x0,%eax
dd:83 c0 01 add$0x1,%eax
e0:a3 00 00 00 00   mov%eax,0x0
  }
e5:83 c4 08 add$0x8,%esp
e8:5d   pop%ebp
e9:c3   ret   
ea:66 0f 1f 44 00 00nopw   0x0(%eax,%eax,1)

But the instructions associated with the statement

  inline_value += 1;

which start at 0xc6, are showing as starting at 0xc3, which is frame
setup code.

With the compiler patch, the prologue record is associated to the first
instruction that is not part of the frame setup code.

  void
  caller_trivial_1 ()
  {
c0:55   push   %ebp
c1:89 e5mov%esp,%ebp
c3:83 ec 08 sub$0x8,%esp
  inline_value += 1;  // At first increment in caller_trivial_1.
c6:a1 00 00 00 00   mov0x0,%eax
cb:83 c0 01 add$0x1,%eax
ce:a3 00 00 00 00   mov%eax,0x0
  caller_trivial_2(); // In caller_trivial_1.
d3:e8 18 00 00 00   call   f0 <_Z16caller_trivial_2v>
  inline_value += 1;
d8:a1 00 00 00 00   mov0x0,%eax
dd:83 c0 01 add$0x1,%eax
e0:a3 00 00 00 00   mov%eax,0x0
  }
e5:83 c4 08 add$0x8,%esp
e8:5d   pop%ebp
e9:c3   ret   
ea:66 0f 1f 44 00 00nopw   0x0(%eax,%eax,1)

Now the instructions associated with 'inline_value += 1;' are correctly
identified and are the same in 0xc6 and 0xd8.


https://reviews.llvm.org/D39283

Files:
  
packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py
  packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp


Index: 
packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp
===
--- packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp
+++ packages/Python/lldbsuite/test/functionalities/inline-stepping/calling.cpp
@@ -68,6 +68,7 @@
 void
 caller_trivial_1 ()
 {
+inline_value += 1;  // At first increment in caller_trivial_1.
 caller_trivial_2(); // In caller_trivial_1.
 inline_value += 1; 
 }
@@ -75,8 +76,9 @@
 void
 caller_trivial_2 ()
 {
+inline_value += 1;  // At first increment in caller_trivial_2.
 inline_trivial_1 (); // In caller_trivial_2.
-inline_value += 1;  // At increment in caller_trivial_2.
+inline_value += 1;  // At second increment in caller_trivial_2.
 }
 
 void
@@ -88,8 +90,9 @@
 void
 inline_trivial_1 ()
 {
+inline_value += 1;  // At first increment in inline_trivial_1.
 inline_trivial_2(); // In inline_trivial_1.
-inline_value += 1;  // At increment in inline_trivial_1.
+inline_value += 1;  // At second increment in inline_trivial_1.
 }
 
 void
Index: 
packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py
===
--- 
packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py
+++ 
packages/Python/lldbsuite/test/functionalities/inline-stepping/TestInlineStepping.py
@@ -183,9 +183,12 @@
 
 # Now step from caller_ref_1 all the way into called_by_inline_trivial
 
-step_sequence = [["// In caller_trivial_1.", "into"],
- ["// In caller_trivial_2.", "into"],
- ["// In inline_trivial_1.", "into"],
+  

[Lldb-commits] [PATCH] D35556: Add data formatter for libc++'s forward_list

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

It looks like the behavior of the two derived list classes here are only 
partially factored out.  See the individual comments but it looks like there is 
a lot more that could be shared.

I also don't think we should change the naming conventions used for these 
classes piecemeal.  If we want to simplify the names and hide the FrontEnds in 
the source files we should do that wholesale, and not one by one.  Otherwise 
next time I read this file I'm going to waste time wondering why the 
ForwardListFrontEnd isn't specific to LibCxx whereas the 
LibcxxStdListSyntheticFrontEnd does seem to be...




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp:5
+{
+  std::forward_list empty{}, one_elt{47}, five_elts{1,22,333,,5};
+  return 0; // break here

Some compilers (including clang at other times) will omit debug info for 
variables that it doesn't see getting used.  I usually try to use the variables 
I'm going to print (call size on them, pass an element to printf, whatever...) 
to keep this from happening.

OTOH, it's nice if compilers don't do that, so maybe relying on their not doing 
that in our tests is a useful forcing function???



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:141
 
-namespace lldb_private {
-namespace formatters {
-class LibcxxStdListSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+class ForwardListFrontEnd: public AbstractListFrontEnd {
+public:

I wonder about the decision to move these classes out of 
lldb_private::formatters.  They don't need to be in a globally visible 
namespace, but all the others are.  They also all have a consistent naming 
convention, which this one doesn't have.  This doesn't seem like the sort of 
thing we should change piecemeal, that will just lead to confusion.

Was there some other reason for doing this that I'm missing?



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:153
+  std::map m_iterators;
+};
+

Why are these three ivars not in the AbstractListFrontEnd? They appear in both 
derived classes and seem to be used in the same way.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:182
+  m_head = nullptr;
+  return false;
 }

The AbstractFrontEndList has the m_fast_runner and m_slow_runner but they are 
only reset in the Update for the LibcxxStdListSyntheticFrontEnd, and not here.  
Is that on purpose?



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:251-260
+  ListIterator current(m_head);
+  if (idx > 0) {
+auto cached_iterator = m_iterators.find(idx - 1);
+if (cached_iterator != m_iterators.end()) {
+  current = cached_iterator->second;
+  actual_advance = 1;
+}

This use of the iterators also seems like it should be shared.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:299-301
+  /*m_node_address = backend_addr->GetValueAsUnsigned(0);
+  if (!m_node_address || m_node_address == LLDB_INVALID_ADDRESS)
+return false;*/

?



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:303-310
+  CompilerType list_type = m_backend.GetCompilerType();
+  if (list_type.IsReferenceType())
+list_type = list_type.GetNonReferenceType();
+
+  if (list_type.GetNumTemplateArguments() == 0)
+return false;
+  TemplateArgumentKind kind;

This bit is done in exactly the same way in the two sub-classes.  Could it be 
moved into the base class Update?


https://reviews.llvm.org/D35556



___
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.

2017-10-25 Thread 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] [lldb] r316609 - Move StopInfoOverride callback to the new architecture plugin

2017-10-25 Thread Pavel Labath via lldb-commits
Author: labath
Date: Wed Oct 25 14:05:31 2017
New Revision: 316609

URL: http://llvm.org/viewvc/llvm-project?rev=316609&view=rev
Log:
Move StopInfoOverride callback to the new architecture plugin

This creates a new Architecture plugin and moves the stop info override
callback to this place. The motivation for this is to remove complex
dependencies from the ArchSpec class because it is used in a lot of
places that (should) know nothing about Process instances and StopInfo
objects.

I also add a test for the functionality covered by the override
callback.

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

Added:
lldb/trunk/include/lldb/Core/Architecture.h
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/Makefile

lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/main.c
lldb/trunk/source/Plugins/Architecture/
lldb/trunk/source/Plugins/Architecture/Arm/
lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.cpp
lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.h
lldb/trunk/source/Plugins/Architecture/Arm/CMakeLists.txt
lldb/trunk/source/Plugins/Architecture/CMakeLists.txt
Modified:
lldb/trunk/include/lldb/Core/ArchSpec.h
lldb/trunk/include/lldb/Core/PluginManager.h
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/API/SystemInitializerFull.cpp
lldb/trunk/source/Core/ArchSpec.cpp
lldb/trunk/source/Core/PluginManager.cpp
lldb/trunk/source/Plugins/CMakeLists.txt
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Target/Target.cpp
lldb/trunk/source/Target/Thread.cpp

Modified: lldb/trunk/include/lldb/Core/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ArchSpec.h?rev=316609&r1=316608&r2=316609&view=diff
==
--- lldb/trunk/include/lldb/Core/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Core/ArchSpec.h Wed Oct 25 14:05:31 2017
@@ -15,6 +15,7 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
+#include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h" // for StringRef
 #include "llvm/ADT/Triple.h"
 
@@ -24,19 +25,6 @@
 #include  // for uint32_t
 
 namespace lldb_private {
-class Platform;
-}
-namespace lldb_private {
-class Stream;
-}
-namespace lldb_private {
-class StringList;
-}
-namespace lldb_private {
-class Thread;
-}
-
-namespace lldb_private {
 
 //--
 /// @class ArchSpec ArchSpec.h "lldb/Core/ArchSpec.h"
@@ -258,8 +246,6 @@ public:
 
   };
 
-  typedef void (*StopInfoOverrideCallbackType)(lldb_private::Thread &thread);
-
   //--
   /// Default constructor.
   ///
@@ -574,34 +560,11 @@ public:
   //--
   bool IsCompatibleMatch(const ArchSpec &rhs) const;
 
-  //--
-  /// Get a stop info override callback for the current architecture.
-  ///
-  /// Most platform specific code should go in lldb_private::Platform,
-  /// but there are cases where no matter which platform you are on
-  /// certain things hold true.
-  ///
-  /// This callback is currently intended to handle cases where a
-  /// program stops at an instruction that won't get executed and it
-  /// allows the stop reasonm, like "breakpoint hit", to be replaced
-  /// with a different stop reason like "no stop reason".
-  ///
-  /// This is specifically used for ARM in Thumb code when we stop in
-  /// an IT instruction (if/then/else) where the instruction won't get
-  /// executed and therefore it wouldn't be correct to show the program
-  /// stopped at the current PC. The code is generic and applies to all
-  /// ARM CPUs.
-  ///
-  /// @return NULL or a valid stop info override callback for the
-  /// current architecture.
-  //--
-  StopInfoOverrideCallbackType GetStopInfoOverrideCallback() const;
-
   bool IsFullySpecifiedTriple() const;
 
   void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different,
   bool &vendor_different, bool &os_different,
-  bool &os_version_different, bool &env_different);
+  bool &os_version_different, bool &env_different) 
const;
 
   //--
   /// Detect whether this architecture uses thumb code exclusively

Added: lldb/trunk/include/lldb/Core/Architecture.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Arc

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-25 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316609: Move StopInfoOverride callback to the new 
architecture plugin (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D31172?vs=106600&id=120309#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31172

Files:
  lldb/trunk/include/lldb/Core/ArchSpec.h
  lldb/trunk/include/lldb/Core/Architecture.h
  lldb/trunk/include/lldb/Core/PluginManager.h
  lldb/trunk/include/lldb/Target/Process.h
  lldb/trunk/include/lldb/Target/Target.h
  lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py
  lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/main.c
  lldb/trunk/source/API/SystemInitializerFull.cpp
  lldb/trunk/source/Core/ArchSpec.cpp
  lldb/trunk/source/Core/PluginManager.cpp
  lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.cpp
  lldb/trunk/source/Plugins/Architecture/Arm/ArchitectureArm.h
  lldb/trunk/source/Plugins/Architecture/Arm/CMakeLists.txt
  lldb/trunk/source/Plugins/Architecture/CMakeLists.txt
  lldb/trunk/source/Plugins/CMakeLists.txt
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/source/Target/Target.cpp
  lldb/trunk/source/Target/Thread.cpp

Index: lldb/trunk/include/lldb/Core/PluginManager.h
===
--- lldb/trunk/include/lldb/Core/PluginManager.h
+++ lldb/trunk/include/lldb/Core/PluginManager.h
@@ -10,6 +10,7 @@
 #ifndef liblldb_PluginManager_h_
 #define liblldb_PluginManager_h_
 
+#include "lldb/Core/Architecture.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"  // for Status
 #include "lldb/lldb-enumerations.h"   // for ScriptLanguage
@@ -54,6 +55,21 @@
   GetABICreateCallbackForPluginName(const ConstString &name);
 
   //--
+  // Architecture
+  //--
+  using ArchitectureCreateInstance =
+  std::unique_ptr (*)(const ArchSpec &);
+
+  static void RegisterPlugin(const ConstString &name,
+ llvm::StringRef description,
+ ArchitectureCreateInstance create_callback);
+
+  static void UnregisterPlugin(ArchitectureCreateInstance create_callback);
+
+  static std::unique_ptr
+  CreateArchitectureInstance(const ArchSpec &arch);
+
+  //--
   // Disassembler
   //--
   static bool RegisterPlugin(const ConstString &name, const char *description,
Index: lldb/trunk/include/lldb/Core/Architecture.h
===
--- lldb/trunk/include/lldb/Core/Architecture.h
+++ lldb/trunk/include/lldb/Core/Architecture.h
@@ -0,0 +1,43 @@
+//===-- Architecture.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLDB_CORE_ARCHITECTURE_H
+#define LLDB_CORE_ARCHITECTURE_H
+
+#include "lldb/Core/PluginInterface.h"
+
+namespace lldb_private {
+
+class Architecture : public PluginInterface {
+public:
+  Architecture() = default;
+  virtual ~Architecture() = default;
+
+  //--
+  /// This is currently intended to handle cases where a
+  /// program stops at an instruction that won't get executed and it
+  /// allows the stop reasonm, like "breakpoint hit", to be replaced
+  /// with a different stop reason like "no stop reason".
+  ///
+  /// This is specifically used for ARM in Thumb code when we stop in
+  /// an IT instruction (if/then/else) where the instruction won't get
+  /// executed and therefore it wouldn't be correct to show the program
+  /// stopped at the current PC. The code is generic and applies to all
+  /// ARM CPUs.
+  //--
+  virtual void OverrideStopInfo(Thread &thread) = 0;
+
+private:
+  Architecture(const Architecture &) = delete;
+  void operator=(const Architecture &) = delete;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_CORE_ARCHITECTURE_H
Index: lldb/trunk/include/lldb/Core/ArchSpec.h
===
--- lldb/trunk/include/lldb/Core/ArchSpec.h
+++ lldb/trunk/include/lldb/Core/ArchSpec.h
@@ -15,6 +15,7 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
+#include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h" // for StringRef
 #include "llvm/ADT/Triple.h"
 
@@ -2

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

2017-10-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Looks good. A little bit of cleanup. Let me know what you think of the comments.




Comment at: utils/clangdiag.py:17
+import os
+from subprocess import check_output as qx;
+

I would rather just do:

```
import subprocess
```

Then later use:

```
subprocess.check_output(...)
```

It makes the code easier to read.



Comment at: utils/clangdiag.py:20
+# State
+Breakpoints = []
+target = None

Remove? See inlined comment for line 92



Comment at: utils/clangdiag.py:21
+Breakpoints = []
+target = None
+diagtool = None

I would avoid making this a global. It will keep the target alive since it has 
a strong reference count to the underlying lldb_private::Target.



Comment at: utils/clangdiag.py:52
+
+global target
+global diagtool

Get the target from the frame instead of grabbing it from a global:
```
target = frame.thread.process.target
```
or without the shortcuts:
```
target = frame.GetThread().GetProcess().GetTarget()
```



Comment at: utils/clangdiag.py:66
+
+global target
+global diagtool

Always grab the target and don't store it in a global. so just remove this line



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

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)
```



Comment at: utils/clangdiag.py:80
+
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')

Add name to breakpoint? See inlined comment at line 92



Comment at: utils/clangdiag.py:87
+def disable(debugger):
+global target
+global diagtool

remove and use:
```
target = debugger.GetSelectedTarget()
```



Comment at: utils/clangdiag.py:91-92
+# Remove all diag breakpoints.
+for bp in Breakpoints:
+target.BreakpointDelete(bp.GetID())
+target = None

Another way to do this would be to give the breakpoints you create using 
"target.BreakpointCreateXXX" to have a name when you create them:

```
bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
bp.AddName("llvm::Diagnostic")
```

Then here you can iterate over all breakpoints in the target:

```
for bp in target.breakpoint_iter():
  if bp.MatchesName("llvm::Diagnostic"):
target.BreakpointDelete(bp.GetID())
```

Then you don't need a global list?



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] D39283: [lldb-dev] Update LLDB test cases for 'inlineStepping'

2017-10-25 Thread Tamas Berghammer via Phabricator via lldb-commits
tberghammer added a comment.

Hi Carlos,

Sorry for not responding to your related e-mails lately. I am still not 
convinced that tis is the right way forward as you are changing the expected 
behavior from LLDB side to make it work with the new debug info emitted by 
clang but I think the current behavior of LLDB is correct in this context. Both 
your reasoning and your compiler fix makes perfect sense for the case you 
mentioned in the commit message (I assume you missed a "inline_value += 1;" 
line from the C code snippet based on the assembly) but I am still concerned by 
the behavior in the original case with the following source code:

  void caller_trivial_1() {
  caller_trivial_2(); // In caller_trivial_1.
  inline_value += 1; 
  }

I think the following debug info is emitted in 32bit mode after your change:

- debug_info: caller_trivial_1 starts at 0x4000
- debug_line: 0x4000 belongs to line X (declaration of caller_trivial_1) and it 
is prologue
- debug_info: inlined caller_trivial_2 starts at 0x4010
- debug_line: 0x4020 belongs to line Y (first line of function 
caller_trivial_1) and it is not prologue
- debug_info: inlined caller_trivial_2 ends at 0x4080

When we step into caller_trivial_1 lldb will step to the first non-prologue 
line entry after the start of f what will be at 0x4020 and when we stop there 
that address is within g so lldb displays that we are stopped in 
caller_trivial_2.

I think the correct debug info would be the following:

- debug_info: caller_trivial_1 starts at 0x4000
- debug_line: 0x4000 belongs to line X (declaration of caller_trivial_1) and it 
is prologue
- debug_line: 0x4010 belongs to line Y (first line of function 
caller_trivial_1) and it is not prologue
- debug_info: inlined caller_trivial_2 starts at 0x4010
- debug_line: 0x4020 belongs to line Y (first line of function 
caller_trivial_2) and it is not prologue
- debug_info: inlined caller_trivial_2 ends at 0x4080

**In case of non-optimized build (it is true for the test) I would expect from 
a compiler that no line entry pointing to a line inside caller_trivial_1 will 
have an address what is inside the address range of any other function inlined 
into caller_trivial_2. I think this held before your change but not after.** 
Can you and others comment on this last expectation? If we agree that this 
expectation should hold then I think changing the test is the wrong and instead 
we should either change the line entry or the inlined function range writing 
code to produce debug info satisfying it.


https://reviews.llvm.org/D39283



___
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

2017-10-25 Thread 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

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

Use the form of the command that gets an SBExecutionContext, then you can avoid 
having to cache the target at all.




Comment at: utils/clangdiag.py:98-100
+def the_diag_command(debugger, command, result, dict):
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would

If you use the form of the command function that takes an execution context:

def command_function(debugger, command, exe_ctx, result, internal_dict):

then you can grab the target from there when the command gets invoked and pass 
it to your enable & disable funcs.  That way you won't have to rely on 
GetSelectedTarget.  That's important, for instance, if you were running a debug 
session with two targets and you wanted to invoke your command in a breakpoint 
command of a breakpoint in target A.  There's no guarantee when target A hits 
the breakpoint that A is the currently selected target (it won't get selected 
till it actually decides  to stop.)  But when the breakpoint runs its command, 
it sets the right target, & thread in the execution context that gets passed in.


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

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Yes definitely use names for your breakpoints.  It makes them easier to handle. 
 Note starting a month or two ago you can set names to not get deleted except 
by an explicit gesture.  That hasn't shown up in releases yet, but once you can 
rely on its being there, you can set the names to not get auto-deleted or 
disabled, and then if somebody does:

(lldb) break disable

not thinking they are affecting your utility, you won't get messed up by this.




Comment at: utils/clangdiag.py:87
+def disable(debugger):
+global target
+global diagtool

clayborg wrote:
> remove and use:
> ```
> target = debugger.GetSelectedTarget()
> ```
See my comment.  Don't use selected targets, use the command form that takes an 
SBExecutionContext.  That's been around for more than a couple of years now, so 
it's pretty safe to use.



Comment at: utils/clangdiag.py:91-92
+# Remove all diag breakpoints.
+for bp in Breakpoints:
+target.BreakpointDelete(bp.GetID())
+target = None

clayborg wrote:
> Another way to do this would be to give the breakpoints you create using 
> "target.BreakpointCreateXXX" to have a name when you create them:
> 
> ```
> bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
> bp.AddName("llvm::Diagnostic")
> ```
> 
> Then here you can iterate over all breakpoints in the target:
> 
> ```
> for bp in target.breakpoint_iter():
>   if bp.MatchesName("llvm::Diagnostic"):
> target.BreakpointDelete(bp.GetID())
> ```
> 
> Then you don't need a global list?
> 
If you name them you can just do:

target.BreakpointDeleteByName("llvm::Diagnostic")

That's even simpler.


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

2017-10-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: utils/clangdiag.py:62
+
+def enable(debugger, args):
+# Always disable existing breakpoints

Pass exe_ctx or target into this instead of the debugger (see Jim's comment on 
execution contexts below.



Comment at: utils/clangdiag.py:78
+print('diagtool not found along side %s' % exe)
+return
+

No need. just a suggestion. Is this information available in the main 
executable as any kind of debug info? If so you can read it from there. If it 
is always in a stand alone separate file, then what you have is ok.



Comment at: utils/clangdiag.py:87
+def disable(debugger):
+global target
+global diagtool

jingham wrote:
> clayborg wrote:
> > remove and use:
> > ```
> > target = debugger.GetSelectedTarget()
> > ```
> See my comment.  Don't use selected targets, use the command form that takes 
> an SBExecutionContext.  That's been around for more than a couple of years 
> now, so it's pretty safe to use.
Just pass the target or exe_ctx into this function then instead of "debugger".



Comment at: utils/clangdiag.py:92
+for bp in Breakpoints:
+target.BreakpointDelete(bp.GetID())
+target = None

nice!



Comment at: utils/clangdiag.py:100
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would
+command_args = shlex.split(command)

Great idea. Forgot about the execution context variant. The "exe_ctx" is a 
lldb.SBExecutionContext object. You get your target using:

```
target = exe_ctx.GetTarget()
```



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

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Note, BTW, Enrico also added a form of the command add that allows you to use a 
Python class to wrap the callable.  That was in early 2015 so it's probably 
safe to use as well by now.  That's even more convenient, and obviates the need 
for globals at all.  One instance of the class is made per debugger as the 
command gets loaded, so it's ivars live for the life of the command - spanning 
execution.

There's an example of it here:

http://llvm.org/svn/llvm-project/lldb/trunk/examples/python/disassembly_mode.py

and it is mentioned in the Python Ref.

You don't by any means have to use this form, but if you are having fun playing 
around with this stuff, it makes it much more pleasant to write commands.




Comment at: utils/clangdiag.py:100
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would
+command_args = shlex.split(command)

clayborg wrote:
> Great idea. Forgot about the execution context variant. The "exe_ctx" is a 
> lldb.SBExecutionContext object. You get your target using:
> 
> ```
> target = exe_ctx.GetTarget()
> ```
> 
Yeah, if I could think of a friendly way to do it, I would outlaw the older 
form.  That was an oversight, and makes commands that aren't guaranteed to 
behave correctly in breakpoint callbacks.

I'll go make the docs a little stronger now that the exe_ctx form has been 
there long enough that it's generally safe to use.


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] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai created this revision.

With this patch I want to do the following:
When searching for global data symbols, only consider non-externally visible
symbols if we are in the module where they are visible.

I was investigating bug 35043
(https://bugs.llvm.org/show_bug.cgi?id=35043)
and found out I was able to reproduce this on a Linux x86-64 machine I have
access to. I was doing some digging, and found out that even when we're not in
the module with visibility, we would still consider internal symbols from other
modules when clang was asking us to find external declarations by name. 
So in TestLambdas, we want to define a lambda with parameters (int a, int b).
Clang wants to ensure we don't redeclare our parameters. Through a long chain of
function calls, the call chain ends up
at SymbolContext::FindBestGlobalDataSymbol(), which (on my machine) will find
that there is are symbols named "a" and "b" in another module, but they are not
externally visible. lldb will complain that there are multiple defined internal
symbols, but these shouldn't matter for our lambda.

I don't have as much context as some of y'all about whether or not this kind of
change will present significant problems elsewhere, or if we should tackle this
problem another way. Feedback is very much appreciated.


https://reviews.llvm.org/D39307

Files:
  
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  source/Symbol/SymbolContext.cpp

Index: source/Symbol/SymbolContext.cpp
===
--- source/Symbol/SymbolContext.cpp
+++ source/Symbol/SymbolContext.cpp
@@ -802,156 +802,152 @@
 const Symbol *
 SymbolContext::FindBestGlobalDataSymbol(const ConstString &name, Status &error) {
   error.Clear();
-  
+
   if (!target_sp) {
 return nullptr;
   }
-  
+
   Target &target = *target_sp;
   Module *module = module_sp.get();
-  
-  auto ProcessMatches = [this, &name, &target, module]
-  (SymbolContextList &sc_list, Status &error) -> const Symbol* {
-llvm::SmallVector external_symbols;
-llvm::SmallVector internal_symbols;
+
+  auto ProcessMatches = [this, &name, &target,
+ module](SymbolContextList &sc_list, Status &error,
+ bool external_only) -> const Symbol * {
+llvm::SmallVector symbols;
 const uint32_t matches = sc_list.GetSize();
 for (uint32_t i = 0; i < matches; ++i) {
   SymbolContext sym_ctx;
   sc_list.GetContextAtIndex(i, sym_ctx);
   if (sym_ctx.symbol) {
 const Symbol *symbol = sym_ctx.symbol;
 const Address sym_address = symbol->GetAddress();
-
+
 if (sym_address.IsValid()) {
   switch (symbol->GetType()) {
-case eSymbolTypeData:
-case eSymbolTypeRuntime:
-case eSymbolTypeAbsolute:
-case eSymbolTypeObjCClass:
-case eSymbolTypeObjCMetaClass:
-case eSymbolTypeObjCIVar:
-  if (symbol->GetDemangledNameIsSynthesized()) {
-// If the demangled name was synthesized, then don't use it
-// for expressions. Only let the symbol match if the mangled
-// named matches for these symbols.
-if (symbol->GetMangled().GetMangledName() != name)
-  break;
-  }
+  case eSymbolTypeData:
+  case eSymbolTypeRuntime:
+  case eSymbolTypeAbsolute:
+  case eSymbolTypeObjCClass:
+  case eSymbolTypeObjCMetaClass:
+  case eSymbolTypeObjCIVar:
+if (symbol->GetDemangledNameIsSynthesized()) {
+  // If the demangled name was synthesized, then don't use it
+  // for expressions. Only let the symbol match if the mangled
+  // named matches for these symbols.
+  if (symbol->GetMangled().GetMangledName() != name)
+break;
+}
+if (external_only) {
   if (symbol->IsExternal()) {
-external_symbols.push_back(symbol);
-  } else {
-internal_symbols.push_back(symbol);
+symbols.push_back(symbol);
   }
-  break;
-case eSymbolTypeReExported: {
-  ConstString reexport_name = symbol->GetReExportedSymbolName();
-  if (reexport_name) {
-ModuleSP reexport_module_sp;
-ModuleSpec reexport_module_spec;
-reexport_module_spec.GetPlatformFileSpec() =
-symbol->GetReExportedSymbolSharedLibrary();
-if (reexport_module_spec.GetPlatformFileSpec()) {
-  reexport_module_sp =
-  target.GetImages().FindFirstModule(reexport_module_spec);
-  if (!reexport_module_sp) {
-reexport_module_spec.GetPlatformFileSpec()
-.GetDirectory()
-.Clear();
- 

[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

It is a nice idea. I would still rather fix this in clang so it doesn't ask us 
about a variable it already knows about. Though this might be a good solution 
until we can fix it for real though. Sean or Jim?


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In https://reviews.llvm.org/D39307#907220, @clayborg wrote:

> It is a nice idea. I would still rather fix this in clang so it doesn't ask 
> us about a variable it already knows about. Though this might be a good 
> solution until we can fix it for real though. Sean or Jim?


From what I can tell, in clang it looks like it's looking for a redeclaration 
of parameters (something like `int foo(int x, int x)`). Not entirely sure why 
it does that, and I'm not entirely sure why it would need to look beyond the 
Function Declaration scope to do that. When I get a chance, I can look into it 
more.


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

Thanks, I'll try this patch tomorrow.
I know this may be a little off, but how hard is to write a test for this so 
that it doesn't regress?


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

If I have a library that uses some singleton class that's not exported, and I'm 
debugging code in a client of my library, and the state of my library is 
looking odd based on what it is returning to the client, it seems not unnatural 
to want to call:

(lldb) expr my_library_singleton->WFTDude()

accessing some debugging function I've put in for that purpose.  This change 
would make that not work.

Even worse, it would work when I stepped into the code of my library, but then 
when I stepped out to the client it would stop working.  And the error would be 
just "unknown symbol my_library_singleton".  We wouldn't have a good way to 
help explain this failure.

If the general vote is "Jim is nuts, no programmer would ever do this" then I 
guess it is okay, but this seems to me a not-implausible debugging scenario, 
and I'd really rather not break it unless we can't think of any other way 
around the problem this is actually trying to solve.


https://reviews.llvm.org/D39307



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


Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Greg Clayton via lldb-commits
Not hard. Just find a test that has a shared library and copy it. Modify the 
shared library to create a symbol named "a". Then the main executable, just run 
the same lambda expression and it will fail on all platforms. The current fix 
in the review is just for internal symbols in other shared libraries, so don't 
make "a" public if you want it to pass. Did you say that "a" in libmath was 
public?

Greg

> On Oct 25, 2017, at 4:10 PM, Davide Italiano via Phabricator 
>  wrote:
> 
> davide added a comment.
> 
> Thanks, I'll try this patch tomorrow.
> I know this may be a little off, but how hard is to write a test for this so 
> that it doesn't regress?
> 
> 
> https://reviews.llvm.org/D39307
> 
> 
> 

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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In https://reviews.llvm.org/D39307#907253, @jingham wrote:

> If I have a library that uses some singleton class that's not exported, and 
> I'm debugging code in a client of my library, and the state of my library is 
> looking odd based on what it is returning to the client, it seems not 
> unnatural to want to call:
>
> (lldb) expr my_library_singleton->WFTDude()
>
> accessing some debugging function I've put in for that purpose.  This change 
> would make that not work.


While not implausible, it seems as though making `my_library_singleton` visible 
from a scope that it is not intended to be visible from is causing the problem 
I am trying to fix. If your library is acting strange while interacting with 
your client, it seems like it's your library that you're really interested in 
debugging (or rather, your library interacting with the client), in which case 
you should probably enter the library's scope and then try to query its 
internals. While what you want to do is quite convenient, it possibly 
introduces more problems than it aims to alleviate.

However, I don't see a problem with adding a flag to expr to say "I want 
visibility from the scope of this module/library/whatever". In that case, we 
could create an expression in some dummy function within the context of the 
module we care about and use that to interact with clang. I'm not sure how 
feasible this would be, nor how much work it would involve. It would be a less 
convenient than what exists now, but it would still be more convenient than 
trying to put a breakpoint somewhere in your library and potentially waiting 
until its too late to gather any meaningful information.

What do you think?

In https://reviews.llvm.org/D39307#907253, @jingham wrote:

> Even worse, it would work when I stepped into the code of my library, but 
> then when I stepped out to the client it would stop working. And the error 
> would be just "unknown symbol my_library_singleton". We wouldn't have a good 
> way to help explain this failure


I totally agree. The diagnostics of failure here are in poor, at best. I could 
modify what we do to say "hey, I found the symbol you're talking about in this 
module, but it's not visible from this context." If my idea about the flag 
above sounds good, we could even say "you can use this flag if want to see it 
here and now".


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In https://reviews.llvm.org/D39307#907248, @davide wrote:

> Thanks, I'll try this patch tomorrow.
>  I know this may be a little off, but how hard is to write a test for this so 
> that it doesn't regress?


I don't think this would be terribly difficult, and I absolutely will write a 
test so we don't regress on this behavior. I wanted to get feedback on my 
change before I try to test its behavior.


https://reviews.llvm.org/D39307



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


Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via lldb-commits
$ nm /lib64/libm.so.6| grep " a$" 
00093bb0 r a
000c6a80 r a
00093bb0 r a

No, they are internal to libm only.

Alex

On 10/25/17, 4:15 PM, "Greg Clayton"  wrote:

Not hard. Just find a test that has a shared library and copy it. Modify 
the shared library to create a symbol named "a". Then the main executable, just 
run the same lambda expression and it will fail on all platforms. The current 
fix in the review is just for internal symbols in other shared libraries, so 
don't make "a" public if you want it to pass. Did you say that "a" in libmath 
was public?

Greg

> On Oct 25, 2017, at 4:10 PM, Davide Italiano via Phabricator 
 wrote:
> 
> davide added a comment.
> 
> Thanks, I'll try this patch tomorrow.
> I know this may be a little off, but how hard is to write a test for this 
so that it doesn't regress?
> 
> 
> 
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D39307&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=6r-mVtAxjRKWgeciEWgXiA&m=h9hoe4kEfRfn96OufBXdCIS_3py_y-MLJPBCpogA1To&s=ZxOsS7LJ5gkx_eYT2WwcoQs2g7pbaXOuzV1pTgBV_Mw&e=
> 
> 
> 



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

Thanks for taking care of the test.

About the libmath question. I thought libmath exported a symbol named `a` ,but 
I haven't checked the `readelf` output.
This is what I saw for the symbols:

  (lldb) image lookup --address 0x777ec290
  Address: libm.so.6[0x000a4290] (libm.so.6..rodata + 186832)
  Summary: libm.so.6`a

But I think there may be another latent bug because what I see here is:

  $ objdump -t libm.so |grep ' a'
  00040040 l O .rodata0008  a11.8714
  00040030 l O .rodata0008  a9.8712
  00040020 l O .rodata0008  a7.8710
  00040010 l O .rodata0008  a5.8708
  00040008 l O .rodata0008  aa5.8709
  0004 l O .rodata0008  a3.8706
  0003fff8 l O .rodata0008  aa3.8707
  00040080 l O .rodata0008  a27.8724
  00040088 l O .rodata0008  a25.8723
  00040078 l O .rodata0008  a23.8722
  00040070 l O .rodata0008  a21.8721
  00040068 l O .rodata0008  a19.8720
  00040060 l O .rodata0008  a17.8719
  00040058 l O .rodata0008  a15.8718
  00040050 l O .rodata0008  a13.8716
  00040048 l O .rodata0008  aa13.8717
  00040038 l O .rodata0008  aa11.8715
  00040028 l O .rodata0008  aa9.8713
  00040018 l O .rodata0008  aa7.8711
  00010ee0  wF .text  00e5  atan2
  0002eb60  wF .text  0009  atanl
  00021ea0  wF .text  0032  atanf
  00010fd0  wF .text  007b  atanh
  00030090  wF .text  0068  acosl
  00010e10  wF .text  0054  acosh
  000230d0  wF .text  0063  acosf
  00010da0  wF .text  0063  acos
  9570  wF .text  0032  atan
  00010e70  wF .text  0063  asin
  00023300  wF .text  007b  atanhf
  000302a0  wF .text  007d  atanhl
  00021da0  wF .text  00f1  asinhf
  0002ea60  wF .text  00f1  asinhl
  00023140  wF .text  0057  acoshf
  00030100  wF .text  0059  acoshl
  9470  wF .text  00f1  asinh
  00030160  wF .text  0068  asinl
  000231a0  wF .text  0063  asinf
  000301d0  wF .text  00cd  atan2l
  00023210  wF .text  00e5  atan2f

So, there' no `a` exported, but a bunch of symbols starting with `a` and then 
followed by a decimal.
If you want, I may provide the shared object (or probably it's easier for you 
to try on Fedora yourself).


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

Sorry, my bad, I haven't grepped properly, there are 3 internal symbols named 
`a` in `libm.so`.


https://reviews.llvm.org/D39307



___
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

2017-10-25 Thread 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] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

  $ nm /lib64/libm.so.6 | grep " a$"
  00093bb0 r a
  000c6a80 r a
  00093bb0 r a

This is what I get, which seems to match what you said. These are definitely 
internal to libm. It's a similar story for the symbol `b`.


https://reviews.llvm.org/D39307



___
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

2017-10-25 Thread 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] D39307: Fix global data symbol resolution

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

So one of the expectations of people using the debugger is that their whole 
program is available to them wherever they happen to be stopped.  People get 
really upset when we break that fiction.  I've experienced this both in Radar 
and in person...  But in this case, it seems like a reasonable expectation:

Imagine the debugging scenario where I had stepped into my library, run my 
debugging function, stepped out to the client and what got returned didn't make 
sense with the output of the debugging function.  So I want to run it again to 
see what happened.  I certainly don't want to have to navigate back to my 
library to do that, I don't want the program to change state at all and there 
may be no functions of my library still on the stack somewhere.

I think if you make me say:

  (lldb) expr --pretend-module MyLib.dyld -- my_library_singleton->WFTDude()

you will not make me happy.  lldb should not force users to disambiguate things 
that are not ambiguous.  If there were multiple visible my_library_singleton 
symbols floating around, that would be my fault for choosing a bad name, and as 
the debugger user I wouldn't feel too bad about having to do something to 
disambiguate.  But if it isn't lldb shouldn't make me have to do more work to 
specify it.


https://reviews.llvm.org/D39307



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


[Lldb-commits] [lldb] r316629 - Makefile.rules: move CFLAGS_EXTRAS to the end of compile line

2017-10-25 Thread Pavel Labath via lldb-commits
Author: labath
Date: Wed Oct 25 16:56:17 2017
New Revision: 316629

URL: http://llvm.org/viewvc/llvm-project?rev=316629&view=rev
Log:
Makefile.rules: move CFLAGS_EXTRAS to the end of compile line

This makes sure that any options specified there override generic
compiler options.

This fixes TestBreakpointIt.py

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

Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules?rev=316629&r1=316628&r2=316629&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Wed Oct 25 
16:56:17 2017
@@ -215,18 +215,18 @@ DEBUG_INFO_FLAG ?= -g
 
 CFLAGS ?= $(DEBUG_INFO_FLAG) -O0 -fno-builtin
 ifeq "$(OS)" "Darwin"
-   CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) 
-I$(LLDB_BASE_DIR)include
+   CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) 
-I$(LLDB_BASE_DIR)include
 else
-   CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) 
-I$(LLDB_BASE_DIR)include
+   CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) 
-I$(LLDB_BASE_DIR)include
 endif
 
-CFLAGS += -include $(THIS_FILE_DIR)test_common.h -I$(THIS_FILE_DIR) 
$(ARCH_CFLAGS)
+CFLAGS += -include $(THIS_FILE_DIR)test_common.h -I$(THIS_FILE_DIR) 
$(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 
 # Use this one if you want to build one part of the result without debug 
information:
 ifeq "$(OS)" "Darwin"
-   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) 
$(CFLAGS_EXTRAS) $(ARCH_CFLAGS)
+   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) 
$(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 else
-   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) 
$(CFLAGS_EXTRAS) $(ARCH_CFLAGS)
+   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) 
$(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 endif
 
 ifeq "$(MAKE_DWO)" "YES"


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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Note, BTW, we absolutely need some way to say "this symbol from this library".  
But first of all, if we're going to do this you need to be able to mix & match 
within an expression which you can't do with a flag to expr.   Instead you need 
something like:

(lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol

That syntax is ugly, we should try to think of something better.  But the main 
point is this should only be necessary when lldb can't find a unique symbol.  
When we can no intervention should be required.


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I thought https://reviews.llvm.org/D33083 was supposed to resolve the 
conflicting-symbol-in-another-library issue by giving precedence to the current 
module.  Why doesn't that apply here?


https://reviews.llvm.org/D39307



___
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

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Ack, my bad.  I should remember not to rely on my memory...

I thought at one point I was going to do it that way, then got annoyed I'd have 
to have "BreakpointDisableByName" etc...  So apparently I made:

SBTarget.FindBreakpointByName

that takes a name & an SBBreakpointList, and the list gets filled with the 
breakpoints that match that name.


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] D39307: Fix global data symbol resolution

2017-10-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

The problem here is that the name that started the whole query is a local 
variable in the code we're currently compiling.  So clang doesn't need to ask 
us about it, and in fact since we're still in mid-compilation lldb doesn't know 
anything about the name we're actually trying to look up.

Actually clang shouldn't be asking us about it at all.  It already knows what 
it is, and it is a local variable so it should know that it takes priority over 
anything else we might find.  But it does, by bad luck we find ONE unambiguous 
symbol  with the same name.  We return that to clang and then it says that 
symbol and the local variable it should never have looked elsewhere for collide.

This fix would only fix one particular version of the problem.  For instance if 
the symbol that was causing problems happened to be external and not internal, 
this fix wouldn't work.  It's just that libm happens to have an internal symbol 
with a quite unfortunate name: 'a'...


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In https://reviews.llvm.org/D39307#907302, @jingham wrote:

> Note, BTW, we absolutely need some way to say "this symbol from this 
> library".  But first of all, if we're going to do this you need to be able to 
> mix & match within an expression which you can't do with a flag to expr.   
> Instead you need something like:
>
> (lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol
>
> That syntax is ugly, we should try to think of something better.  But the 
> main point is this should only be necessary when lldb can't find a unique 
> symbol.  When we can no intervention should be required.


I see what you mean. I think I'd agree with you here, being able to mix and 
match would be a useful thing to be able to do while debugging. The syntax is 
something we can work on.

In https://reviews.llvm.org/D39307#907317, @jingham wrote:

> Actually clang shouldn't be asking us about it at all.  It already knows what 
> it is, and it is a local variable so it should know that it takes priority 
> over anything else we might find.


Right. We get into this whole mess when clang starts to check for redeclaration 
of parameters in `Sema::ActOnParamDeclaration()` in 
`$clang_root/lib/Sema/SemaDecl.cpp`. It basically takes your parameter and 
calls `LookupName` with it. I'm not sure why it would ever need to look beyond 
the function declaration scope, but that's what it does. It seems to recurse 
through scopes until it hits the TU scope, and from there it tries to find the 
symbol `a`. You can get a rough idea of what it tries to do with this backtrace:

  (lldb) bt
  * thread #1, name = 'lldb', stop reason = breakpoint 1.1
* frame #0: 0x7fada91245a1 
liblldb.so.6`lldb_private::SymbolContext::FindBestGlobalDataSymbol(this=0x007e69b0,
 name=0x7ffe8e8c7810, error=0x7ffe8e8c73c0) at SymbolContext.cpp:804
  frame #1: 0x7fada930b81d 
liblldb.so.6`lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls(this=0x00852b60,
 context=0x7ffe8e8c7e40, module_sp=nullptr, 
namespace_decl=0x7ffe8e8c7cb0, current_id=16) at 
ClangExpressionDeclMap.cpp:1545
  frame #2: 0x7fada9308a2b 
liblldb.so.6`lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls(this=0x00852b60,
 context=0x7ffe8e8c7e40) at ClangExpressionDeclMap.cpp:843
  frame #3: 0x7fada92d3d03 
liblldb.so.6`lldb_private::ClangASTSource::FindExternalVisibleDeclsByName(this=0x00852b60,
 decl_ctx=0x00934078, clang_decl_name=(Ptr = 9898096)) at 
ClangASTSource.cpp:261
  frame #4: 0x7fada90c5c2b 
liblldb.so.6`lldb_private::ClangASTSource::ClangASTSourceProxy::FindExternalVisibleDeclsByName(this=0x00675040,
 DC=0x00934078, Name=(Ptr = 9898096)) at ClangASTSource.h:246
  frame #5: 0x7fadad10a7f1 
liblldb.so.6`clang::DeclContext::lookup(this=0x00934078, Name=(Ptr = 
9898096)) const at DeclBase.cpp:1542
  frame #6: 0x7fadac1c2ffa 
liblldb.so.6`::LookupDirect(S=0x0093d7d0, R=0x7ffe8e8c89c0, 
DC=0x00934078) at SemaLookup.cpp:843
  frame #7: 0x7fadac1c35eb 
liblldb.so.6`::CppNamespaceLookup(S=0x0093d7d0, R=0x7ffe8e8c89c0, 
Context=0x00926ec0, NS=0x00934078, 
UDirs=0x7ffe8e8c8650)::UnqualUsingDirectiveSet &const) at SemaLookup.cpp:942
  frame #8: 0x7fadac1c4490 
liblldb.so.6`clang::Sema::CppLookupName(this=0x0093d7d0, 
R=0x7ffe8e8c89c0, S=0x009464a0) at SemaLookup.cpp:1322
  frame #9: 0x7fadac1c5d44 
liblldb.so.6`clang::Sema::LookupName(this=0x0093d7d0, 
R=0x7ffe8e8c89c0, S=0x0095aa60, AllowBuiltinCreation=false) at 
SemaLookup.cpp:1826
  frame #10: 0x7fadabd285b7 
liblldb.so.6`clang::Sema::ActOnParamDeclarator(this=0x0093d7d0, 
S=0x0095aa60, D=0x7ffe8e8c8e30) at SemaDecl.cpp:11775
  frame #11: 0x7fadab823c62 
liblldb.so.6`clang::Parser::ParseParameterDeclarationClause(this=0x00942260,
 D=0x7ffe8e8c9b50, FirstArgAttrs=0x7ffe8e8ca2a0, 
ParamInfo=0x7ffe8e8c9930, EllipsisLoc=0x7ffe8e8ca230) at 
ParseDecl.cpp:6351
  frame #12: 0x7fadab866d73 
liblldb.so.6`clang::Parser::ParseLambdaExpressionAfterIntroducer(this=0x00942260,
 Intro=0x7ffe8e8ca530) at ParseExprCXX.cpp:1127
  frame #13: 0x7fadab8655fe 
liblldb.so.6`clang::Parser::ParseLambdaExpression(this=0x00942260) at 
ParseExprCXX.cpp:685


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In https://reviews.llvm.org/D39307#907301, @jingham wrote:

> So one of the expectations of people using the debugger is that their whole 
> program is available to them wherever they happen to be stopped.  People get 
> really upset when we break that fiction.  I've experienced this both in Radar 
> and in person...  But in this case, it seems like a reasonable expectation:
>
> Imagine the debugging scenario where I had stepped into my library, run my 
> debugging function, stepped out to the client and what got returned didn't 
> make sense with the output of the debugging function.  So I want to run it 
> again to see what happened.  I certainly don't want to have to navigate back 
> to my library to do that, I don't want the program to change state at all and 
> there may be no functions of my library still on the stack somewhere.
>
> I think if you make me say:
>
>   (lldb) expr --pretend-module MyLib.dyld -- my_library_singleton->WFTDude()
>  
>
>
> you will not make me happy.  lldb should not force users to disambiguate 
> things that are not ambiguous.  If there were multiple visible 
> my_library_singleton symbols floating around, that would be my fault for 
> choosing a bad name, and as the debugger user I wouldn't feel too bad about 
> having to do something to disambiguate.  But if it isn't lldb shouldn't make 
> me have to do more work to specify it.


I think that the problem here is that lldb is disambiguating and grabbing 
internal symbols from libraries I don't want it to. It seems like clang 
shouldn't even be asking us about this in the first place, but I don't fully 
understand why it's trying to do what it's trying to do. Perhaps somebody with 
more understanding of what clang should be doing can shine some light?


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a subscriber: rsmith.
davide added a comment.

Richard Smith (@rsmith) owns clang and is familiar with this.


https://reviews.llvm.org/D39307



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


Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Zachary Turner via lldb-commits
On Wed, Oct 25, 2017 at 4:59 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:

> jingham added a comment.
>
> Note, BTW, we absolutely need some way to say "this symbol from this
> library".  But first of all, if we're going to do this you need to be able
> to mix & match within an expression which you can't do with a flag to
> expr.   Instead you need something like:
>
> (lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol
>
> That syntax is ugly, we should try to think of something better.  But the
> main point is this should only be necessary when lldb can't find a unique
> symbol.  When we can no intervention should be required.
>

+1, this is very useful.  The Microsoft syntax for this is here:

https://docs.microsoft.com/en-us/visualstudio/debugger/context-operator-cpp

Which is pretty nice imo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

2017-10-25 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

This commit implements basic DidAttach and DidLaunch for the windows
DynamicLoader plugin which allow us to load shared libraries from the
inferior.

This is an improved version of https://reviews.llvm.org/D12245 and should work 
much better.


https://reviews.llvm.org/D39314

Files:
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp


Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -10,12 +10,14 @@
 
 #include "DynamicLoaderWindowsDYLD.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPlanStepInstruction.h"
+#include "lldb/Utility/Log.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -60,9 +62,49 @@
   return nullptr;
 }
 
-void DynamicLoaderWindowsDYLD::DidAttach() {}
+void DynamicLoaderWindowsDYLD::DidAttach() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
 
-void DynamicLoaderWindowsDYLD::DidLaunch() {}
+  DidLaunch();
+
+  m_process->LoadModules();
+}
+
+void DynamicLoaderWindowsDYLD::DidLaunch() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
+
+  ModuleSP executable = GetTargetExecutable();
+
+  if (!executable.get())
+return;
+
+  // Try to fetch the load address of the file from the process, since there
+  // could be randomization of the load address.
+
+  // It might happen that the remote has a different dir for the file, so we
+  // only send the basename of the executable in the query. I think this is 
safe
+  // because I doubt that two executables with the same basenames are loaded in
+  // memory...
+  FileSpec file_spec(
+  executable->GetPlatformFileSpec().GetFilename().GetCString(), false);
+  bool is_loaded;
+  addr_t base_addr = 0;
+  lldb::addr_t load_addr;
+  Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, 
load_addr);
+  if (error.Success() && is_loaded) {
+base_addr = load_addr;
+  }
+
+  UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);
+
+  ModuleList module_list;
+  module_list.Append(executable);
+  m_process->GetTarget().ModulesDidLoad(module_list);
+}
 
 Status DynamicLoaderWindowsDYLD::CanLoadImage() { return Status(); }
 


Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -10,12 +10,14 @@
 
 #include "DynamicLoaderWindowsDYLD.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPlanStepInstruction.h"
+#include "lldb/Utility/Log.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -60,9 +62,49 @@
   return nullptr;
 }
 
-void DynamicLoaderWindowsDYLD::DidAttach() {}
+void DynamicLoaderWindowsDYLD::DidAttach() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
 
-void DynamicLoaderWindowsDYLD::DidLaunch() {}
+  DidLaunch();
+
+  m_process->LoadModules();
+}
+
+void DynamicLoaderWindowsDYLD::DidLaunch() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
+
+  ModuleSP executable = GetTargetExecutable();
+
+  if (!executable.get())
+return;
+
+  // Try to fetch the load address of the file from the process, since there
+  // could be randomization of the load address.
+
+  // It might happen that the remote has a different dir for the file, so we
+  // only send the basename of the executable in the query. I think this is safe
+  // because I doubt that two executables with the same basenames are loaded in
+  // memory...
+  FileSpec file_spec(
+  executable->GetPlatformFileSpec().GetFilename().GetCString(), false);
+  bool is_loaded;
+  addr_t base_addr = 0;
+  lldb::addr_t load_addr;
+  Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, load_addr);
+  if (error.Success() && is_loaded) {
+base_addr = load_addr;
+  }
+
+  UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);
+
+  ModuleList module_list;
+  module_list.Append(executable);
+  m_process->GetTarget().ModulesDidLoa

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-25 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.
Herald added subscribers: kristof.beyls, aemerson.

This assumes that the environment is always Thumb

Change by Walter Erquinigo 


https://reviews.llvm.org/D39315

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


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -648,6 +648,16 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  ArchSpec header_arch;
+  GetArchitecture(header_arch);
+  if (header_arch.GetMachine() == llvm::Triple::arm) {
+if (function_rva & 1) {
+  // For Thumb we need the last bit to be 0 so that the address
+  // points to the right beginning of the symbol.
+  function_rva ^= 1;
+}
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -648,6 +648,16 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  ArchSpec header_arch;
+  GetArchitecture(header_arch);
+  if (header_arch.GetMachine() == llvm::Triple::arm) {
+if (function_rva & 1) {
+  // For Thumb we need the last bit to be 0 so that the address
+  // points to the right beginning of the symbol.
+  function_rva ^= 1;
+}
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] Buildbot numbers for the week of 10/8/2017 - 10/14/2017

2017-10-25 Thread Galina Kistanova via lldb-commits
Hello everyone,

Below are some buildbot numbers for the week of 10/8/2017 - 10/14/2017.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:

 buildername |  was_red
-+--
 libunwind-sphinx-docs   | 107:50:11
 clang-x86_64-linux-selfhost-modules-2   | 71:20:57
 clang-x64-ninja-win7| 62:39:00
 clang-ppc64be-linux-multistage  | 48:21:28
 clang-s390x-linux-multistage| 48:14:17
 clang-cmake-aarch64-quick   | 45:18:57
 lld-x86_64-win7 | 40:56:10
 clang-ppc64be-linux-lnt | 40:33:20
 clang-s390x-linux   | 40:28:28
 clang-ppc64be-linux | 39:41:23
 clang-s390x-linux-lnt   | 38:17:16
 llvm-mips-linux | 38:09:27
 lldb-x86_64-ubuntu-14.04-buildserver| 37:50:24
 llvm-clang-x86_64-expensive-checks-win  | 34:29:36
 aosp-O3-polly-before-vectorizer-unprofitable| 24:13:51
 clang-x86-windows-msvc2015  | 22:52:26
 sanitizer-windows   | 20:58:35
 llvm-clang-lld-x86_64-debian-fast   | 19:37:24
 lld-x86_64-darwin13 | 18:39:31
 libcxx-libcxxabi-libunwind-arm-linux| 16:42:34
 clang-atom-d525-fedora-rel  | 16:24:50
 clang-x86_64-linux-abi-test | 16:11:09
 clang-cmake-armv7-a15-selfhost  | 15:42:30
 clang-cmake-armv7-a15-selfhost-neon | 14:25:32
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 13:18:25
 libcxx-libcxxabi-singlethreaded-x86_64-linux-debian | 12:54:06
 clang-ppc64le-linux-multistage  | 12:51:06
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions   | 12:46:40
 clang-x86_64-debian-fast| 12:17:34
 clang-ppc64le-linux-lnt | 12:15:29
 perf-x86_64-penryn-O3-polly-unprofitable| 11:41:30
 clang-cmake-thumbv7-a15-full-sh | 11:23:58
 ubuntu-gcc7.1-werror| 11:02:43
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu  | 10:44:43
 clang-ppc64le-linux | 10:11:51
 clang-with-thin-lto-ubuntu  | 09:52:22
 sanitizer-x86_64-linux  | 09:49:15
 sanitizer-x86_64-linux-bootstrap| 09:43:23
 clang-cmake-aarch64-lld | 09:37:02
 sanitizer-ppc64be-linux | 09:13:46
 clang-with-lto-ubuntu   | 09:06:37
 clang-x86_64-linux-selfhost-modules | 08:30:46
 clang-hexagon-elf   | 08:25:38
 clang-cmake-aarch64-full| 08:03:20
 sanitizer-x86_64-linux-fast | 07:52:47
 clang-with-thin-lto-windows | 07:50:49
 clang-cmake-x86_64-avx2-linux   | 07:28:45
 clang-cmake-x86_64-avx2-linux-perf  | 07:25:49
 clang-native-arm-lnt| 07:24:38
 sanitizer-x86_64-linux-android  | 07:10:52
 libcxx-libcxxabi-libunwind-aarch64-linux| 07:05:36
 clang-lld-x86_64-2stage | 06:33:45
 clang-cmake-armv7-a15-full  | 05:08:33
 libcxx-libcxxabi-x86_64-linux-debian| 04:19:24
 clang-cmake-aarch64-global-isel | 04:19:10
 polly-amd64-linux   | 03:03:25
 lldb-windows7-android   | 02:34:01
 lldb-x86_64-ubuntu-14.04-cmake  | 02:22:42
 sanitizer-x86_64-linux-fuzzer   | 02:09:49
 polly-arm-linux | 02:08:28
 sanitizer-x86_64-linux-autoconf | 02:05:53
 clang-cmake-x86_64-sde-avx512-linux | 02:03:45
 clang-cmake-armv7-a15   | 02:01:31
 clang-cmake-thumbv7-a15 | 02:01:29
 clang-cuda-build| 01:46:15
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 01:42:39
 lldb-amd64-ninja

[Lldb-commits] Buildbot numbers for the last week of 10/15/2017 - 10/21/2017

2017-10-25 Thread Galina Kistanova via lldb-commits
Hello everyone,

Below are some buildbot numbers for the last week of 10/15/2017 -
10/21/2017.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:

  buildername  | was_red
---+-
 aosp-O3-polly-before-vectorizer-unprofitable  | 71:30:28
 ubuntu-gcc7.1-werror  | 68:44:27
 clang-cmake-aarch64-global-isel   | 32:36:07
 polly-arm-linux   | 32:19:12
 polly-amd64-linux | 30:44:01
 llvm-hexagon-elf  | 29:43:53
 clang-cmake-thumbv7-a15   | 28:57:05
 clang-cmake-armv7-a15 | 28:56:29
 clang-cmake-armv7-a15-full| 28:23:41
 clang-cmake-armv7-a15-selfhost| 26:59:39
 clang-cmake-x86_64-avx2-linux-perf| 23:01:17
 lldb-windows7-android | 22:21:07
 clang-lld-x86_64-2stage   | 20:01:03
 lld-x86_64-win7   | 17:40:51
 lld-x86_64-darwin13   | 16:57:29
 sanitizer-x86_64-linux-bootstrap-ubsan| 14:00:10
 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions | 12:02:27
 clang-cmake-armv7-a15-selfhost-neon   | 06:42:18
 clang-cmake-thumbv7-a15-full-sh   | 06:26:56
 llvm-clang-lld-x86_64-debian-fast | 05:37:36
 clang-cmake-aarch64-lld   | 05:01:27
 clang-s390x-linux-multistage  | 04:47:59
 clang-x86_64-debian-fast  | 04:39:05
 clang-with-lto-ubuntu | 04:37:47
 clang-ppc64be-linux-multistage| 04:33:59
 clang-with-thin-lto-ubuntu| 04:28:47
 clang-x86-windows-msvc2015| 04:18:14
 clang-cuda-build  | 04:16:27
 lldb-x86_64-ubuntu-14.04-cmake| 03:54:37
 sanitizer-x86_64-linux-bootstrap  | 03:52:54
 llvm-clang-x86_64-expensive-checks-win| 03:52:38
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast  | 03:52:38
 llvm-mips-linux   | 03:52:24
 clang-cmake-x86_64-avx2-linux | 03:51:44
 clang-ppc64be-linux   | 03:49:07
 clang-x86_64-linux-selfhost-modules   | 03:48:02
 clang-ppc64le-linux-multistage| 03:46:19
 clang-s390x-linux-lnt | 03:42:08
 clang-x86_64-linux-selfhost-modules-2 | 03:40:36
 clang-ppc64be-linux-lnt   | 03:39:46
 clang-cmake-x86_64-sde-avx512-linux   | 03:39:40
 clang-ppc64le-linux   | 03:39:35
 clang-ppc64le-linux-lnt   | 03:39:27
 clang-atom-d525-fedora-rel| 03:39:00
 clang-with-thin-lto-windows   | 03:30:02
 clang-x64-ninja-win7  | 03:28:55
 sanitizer-x86_64-linux-fast   | 03:25:26
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast| 03:19:29
 sanitizer-x86_64-linux-bootstrap-msan | 03:10:22
 libcxx-libcxxabi-libunwind-x86_64-linux-debian| 03:05:55
 libcxx-libcxxabi-libunwind-arm-linux  | 03:04:33
 libcxx-libcxxabi-libunwind-arm-linux-noexceptions | 03:04:21
 libcxx-libcxxabi-libunwind-aarch64-linux  | 03:03:35
 sanitizer-ppc64be-linux   | 03:02:51
 clang-s390x-linux | 03:02:11
 clang-cmake-aarch64-full  | 02:57:52
 sanitizer-windows | 02:48:03
 sanitizer-x86_64-linux| 02:35:56
 lldb-x86-windows-msvc2015 | 02:34:58
 lldb-x86_64-darwin-13.4   | 02:18:57
 perf-x86_64-penryn-O3-polly-unprofitable  | 02:17:35
 clang-x86_64-linux-abi-test   | 01:40:14
 lldb-amd64-ninja-netbsd8  | 01:30:14
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan | 01:27:57
 lldb-x86_

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

2017-10-25 Thread 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.'