Re: [Lldb-commits] [PATCH] D22286: [LLDB] Help text overhaul

2016-07-13 Thread Pavel Labath via lldb-commits 
labath added a comment.

lgtm


http://reviews.llvm.org/D22286



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

Re: [Lldb-commits] [PATCH] D22284: [LLDB] Proposed change in multi-line edit behavior (Return = end/append, Meta+Return = line break)

2016-07-13 Thread Pavel Labath via lldb-commits 
labath added a comment.

looks good as far as I can tell

I second the idea of running clang-format over the patch.


Repository:
  rL LLVM

http://reviews.llvm.org/D22284



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Pavel Labath via lldb-commits 
labath added a subscriber: labath.
labath added a comment.

Mostly just comments about the test from me.



Comment at: include/lldb/Core/Module.h:240
@@ -239,2 +239,3 @@
 FindFirstSymbolWithNameAndType (const ConstString &name, 
+const lldb::SymbolRewriterSP rewriter,
 lldb::SymbolType symbol_type = 
lldb::eSymbolTypeAny);

The const, as specified here is useless. `const SP &` avoids a copy, 
`shared_ptr` ensures the rewriter cannot be modified. You 
probably want one of those.


Comment at: 
packages/Python/lldbsuite/test/lang/c/symbol_rewriter/TestSymbolRewriter.py:20
@@ +19,3 @@
+@expectedFailureAll(compiler="clang", compiler_version=["<", "3.7"])
+@expectedFailureAll(compiler='gcc')
+@skipIfWindows

If these compilers don't even support the features, go with skip instead of 
xfail.


Comment at: 
packages/Python/lldbsuite/test/lang/c/symbol_rewriter/TestSymbolRewriter.py:28
@@ +27,3 @@
+# Clang does not rewrite dwarf debug info, so it must be stripped
+subprocess.check_call(['strip', '-g', exe])
+

This is not going to work for remote targets on different architectures. If you 
don't need debug info, could you just avoid generating it in the first place 
(-g0 ?). Maybe then you would be able to get this working on windows as well.
If it doesn't work, then we should wire up this call to go through the 
Makefile, as it already knows how to find the right toolchain for 
cross-compilation.


http://reviews.llvm.org/D22294



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

[Lldb-commits] [lldb] r275260 - Add "support" for DW_CFA_GNU_args_size to the unwinder

2016-07-13 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Wed Jul 13 05:55:24 2016
New Revision: 275260

URL: http://llvm.org/viewvc/llvm-project?rev=275260&view=rev
Log:
Add "support" for DW_CFA_GNU_args_size to the unwinder

Summary:
This adds the knowledge of the DW_CFA_GNU_args_size instruction to the eh_frame 
parsing code.
Right now it is ignored as I am unsure how is it supposed to be handled, but 
now we are at least
able to parse the rest of the FDE containing this instruction.

I also add a fix for a bug which was exposed by this instruction. Namely, a 
mismatched sequence
of remember/restore instructions in the input could cause us to pop an empty 
stack and crash. Now
we just log the error and ignore the offending instruction.

Reviewers: jasonmolenda

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D22266

Modified:
lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp

Modified: lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp?rev=275260&r1=275259&r2=275260&view=diff
==
--- lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp (original)
+++ lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp Wed Jul 13 05:55:24 2016
@@ -408,6 +408,7 @@ DWARFCallFrameInfo::GetFDEIndex ()
 bool
 DWARFCallFrameInfo::FDEToUnwindPlan (dw_offset_t dwarf_offset, Address 
startaddr, UnwindPlan& unwind_plan)
 {
+Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND);
 lldb::offset_t offset = dwarf_offset;
 lldb::offset_t current_entry = offset;
 
@@ -648,6 +649,15 @@ DWARFCallFrameInfo::FDEToUnwindPlan (dw_
 // the stack and place them in the current row. (This 
operation is
 // useful for compilers that move epilogue code into 
the body of a
 // function.)
+if (stack.empty())
+{
+if (log)
+log->Printf(
+"DWARFCallFrameInfo::%s(dwarf_offset: %" 
PRIx32 ", startaddr: %" PRIx64
+" encountered DW_CFA_restore_state but 
state stack is empty. Corrupt unwind info?",
+__FUNCTION__, dwarf_offset, 
startaddr.GetFileAddress());
+break;
+}
 lldb::addr_t offset = row->GetOffset ();
 row = stack.back ();
 stack.pop_back ();
@@ -655,6 +665,16 @@ DWARFCallFrameInfo::FDEToUnwindPlan (dw_
 break;
 }
 
+case DW_CFA_GNU_args_size: // 0x2e
+{
+// The DW_CFA_GNU_args_size instruction takes an 
unsigned LEB128 operand
+// representing an argument size. This instruction 
specifies the total of
+// the size of the arguments which have been pushed 
onto the stack.
+
+// TODO: Figure out how we should handle this.
+m_cfi_data.GetULEB128(&offset);
+}
+
 case DW_CFA_val_offset  :   // 0x14
 case DW_CFA_val_offset_sf   :   // 0x15
 default:


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

Re: [Lldb-commits] [PATCH] D22266: Add "support" for DW_CFA_GNU_args_size to the unwinder

2016-07-13 Thread Pavel Labath via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL275260: Add "support" for DW_CFA_GNU_args_size to the 
unwinder (authored by labath).

Changed prior to commit:
  http://reviews.llvm.org/D22266?vs=63678&id=63792#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D22266

Files:
  lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp

Index: lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
@@ -408,6 +408,7 @@
 bool
 DWARFCallFrameInfo::FDEToUnwindPlan (dw_offset_t dwarf_offset, Address 
startaddr, UnwindPlan& unwind_plan)
 {
+Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND);
 lldb::offset_t offset = dwarf_offset;
 lldb::offset_t current_entry = offset;
 
@@ -648,13 +649,32 @@
 // the stack and place them in the current row. (This 
operation is
 // useful for compilers that move epilogue code into 
the body of a
 // function.)
+if (stack.empty())
+{
+if (log)
+log->Printf(
+"DWARFCallFrameInfo::%s(dwarf_offset: %" 
PRIx32 ", startaddr: %" PRIx64
+" encountered DW_CFA_restore_state but 
state stack is empty. Corrupt unwind info?",
+__FUNCTION__, dwarf_offset, 
startaddr.GetFileAddress());
+break;
+}
 lldb::addr_t offset = row->GetOffset ();
 row = stack.back ();
 stack.pop_back ();
 row->SetOffset (offset);
 break;
 }
 
+case DW_CFA_GNU_args_size: // 0x2e
+{
+// The DW_CFA_GNU_args_size instruction takes an 
unsigned LEB128 operand
+// representing an argument size. This instruction 
specifies the total of
+// the size of the arguments which have been pushed 
onto the stack.
+
+// TODO: Figure out how we should handle this.
+m_cfi_data.GetULEB128(&offset);
+}
+
 case DW_CFA_val_offset  :   // 0x14
 case DW_CFA_val_offset_sf   :   // 0x15
 default:


Index: lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
@@ -408,6 +408,7 @@
 bool
 DWARFCallFrameInfo::FDEToUnwindPlan (dw_offset_t dwarf_offset, Address startaddr, UnwindPlan& unwind_plan)
 {
+Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND);
 lldb::offset_t offset = dwarf_offset;
 lldb::offset_t current_entry = offset;
 
@@ -648,13 +649,32 @@
 // the stack and place them in the current row. (This operation is
 // useful for compilers that move epilogue code into the body of a
 // function.)
+if (stack.empty())
+{
+if (log)
+log->Printf(
+"DWARFCallFrameInfo::%s(dwarf_offset: %" PRIx32 ", startaddr: %" PRIx64
+" encountered DW_CFA_restore_state but state stack is empty. Corrupt unwind info?",
+__FUNCTION__, dwarf_offset, startaddr.GetFileAddress());
+break;
+}
 lldb::addr_t offset = row->GetOffset ();
 row = stack.back ();
 stack.pop_back ();
 row->SetOffset (offset);
 break;
 }
 
+case DW_CFA_GNU_args_size: // 0x2e
+{
+// The DW_CFA_GNU_args_size instruction takes an unsigned LEB128 operand
+// representing an argument size. This instruction specifies the total of
+// the size of the arguments which have been pushed onto the stack.
+
+// TODO: Figure out how we should handle this.
+m_cfi_data.GetULEB128(&offset);
+}
+
 case DW_CFA_val_offset  :   // 0x14
 case DW_CFA_val_offset_sf   :   // 0x15
 default:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists

Re: [Lldb-commits] [PATCH] D22266: Add "support" for DW_CFA_GNU_args_size to the unwinder

2016-07-13 Thread Pavel Labath via lldb-commits 
labath added a comment.

In http://reviews.llvm.org/D22266#482626, @jasonmolenda wrote:

> This is fine - is there a binary using this?  I'd love to see the assembly 
> code and a dump of the eh_frame CFI.
>
> I googled around and didn't find much about DW_CFA_GNU_args_size but in the 
> libunwind llvm sources (v. 
> https://llvm.org/svn/llvm-project/libunwind/trunk/src/ ) it takes the uleb 
> value and adjusts the stack pointer by that value??  I don't see why that 
> wouldn't be encoded in the normal "CFA is sp + offset" type rule of eh_frame 
> so it must not be quite that simple.  lldb will probably unwind incorrectly 
> if code built frameless (not using a frame pointer register) is encountered.


That's pretty much what I was able to figure out as well. I haven't managed to 
understand it's purpose either.

GCC 4.9 seems to produce this instruction fairly liberally. However, I was 
unable to get it to produce this instruction in an esp-based frame (which could 
mean that we don't need to handle it? I don't know..) In any case, I am 
attaching a reduced object file demonstrating it's use of this function 
F2161596: thread.o , so you should be able to 
get all the info you requested from it. If you are able to help us with this, 
it would be very appreciated. Let me know if there is anything else I can do to 
help you with that.

For reference, the approximate source code of the function which produced that 
attribute is:

  extern "C" void*
  execute_native_thread_routine(void* __p)
  {
thread::_Impl_base* __t = static_cast(__p);
thread::__shared_base_type __local;
__local.swap(__t->_M_this_ptr);
  
{
  __t->_M_run();
}
  
return 0;
  }


Repository:
  rL LLVM

http://reviews.llvm.org/D22266



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

Re: [Lldb-commits] [PATCH] D21757: Fix lldb-mi disable/enable breakpoints commands

2016-07-13 Thread Ilia K via lldb-commits 
ki.stfu added a comment.

Hi Chuck! Sorry for delay, didn't see this CL. I'll check it tomorrow, so pls 
wait one more day.


Repository:
  rL LLVM

http://reviews.llvm.org/D21757



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Francis Ricci via lldb-commits 
fjricci marked 3 inline comments as done.


Comment at: 
packages/Python/lldbsuite/test/lang/c/symbol_rewriter/TestSymbolRewriter.py:28
@@ +27,3 @@
+# Clang does not rewrite dwarf debug info, so it must be stripped
+subprocess.check_call(['strip', '-g', exe])
+

labath wrote:
> This is not going to work for remote targets on different architectures. If 
> you don't need debug info, could you just avoid generating it in the first 
> place (-g0 ?). Maybe then you would be able to get this working on windows as 
> well.
> If it doesn't work, then we should wire up this call to go through the 
> Makefile, as it already knows how to find the right toolchain for 
> cross-compilation.
Unfortunately, the '-g0' override will only work for dwarf tests. However, 
based on TestWithLimitDebugInfo, it looks like we can skip the test for 
non-dwarf symbols (`@skipIf(debug_info=no_match(["dwarf"]))`). I think this is 
probably the cleanest way to go.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Francis Ricci via lldb-commits 
fjricci updated this revision to Diff 63816.
fjricci marked an inline comment as done.
fjricci added a comment.

Fix const SP and update unit test

Will now only run the unit test as a dwarf test (with -g0),
but since we don't want to test the debug data anyway,
this shouldn't be an issue, and avoids the use of strip.


http://reviews.llvm.org/D22294

Files:
  include/lldb/Core/Module.h
  include/lldb/Core/ModuleList.h
  include/lldb/Symbol/SymbolRewriter.h
  include/lldb/Symbol/Symtab.h
  include/lldb/Target/Target.h
  include/lldb/lldb-forward.h
  lldb.xcodeproj/project.pbxproj
  packages/Python/lldbsuite/test/lang/c/symbol_rewriter/Makefile
  packages/Python/lldbsuite/test/lang/c/symbol_rewriter/TestSymbolRewriter.py
  packages/Python/lldbsuite/test/lang/c/symbol_rewriter/main.c
  packages/Python/lldbsuite/test/lang/c/symbol_rewriter/rewrite.map
  source/API/SBModule.cpp
  source/API/SBTarget.cpp
  source/Breakpoint/BreakpointResolverName.cpp
  source/Commands/CommandObjectSource.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/AddressResolverName.cpp
  source/Core/Disassembler.cpp
  source/Core/Module.cpp
  source/Core/ModuleList.cpp
  source/Core/SourceManager.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
  source/Plugins/DynamicLoader/Hexagon-DYLD/HexagonDYLDRendezvous.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp
  
source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
  source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  source/Symbol/CMakeLists.txt
  source/Symbol/Symbol.cpp
  source/Symbol/SymbolRewriter.cpp
  source/Symbol/Symtab.cpp
  source/Target/ObjCLanguageRuntime.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -78,6 +78,7 @@
   m_mutex(),
   m_arch(target_arch),
   m_images(this),
+  m_symbol_rewriter(),
   m_section_load_history(),
   m_breakpoint_list(false),
   m_internal_breakpoint_list(true),
Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -105,6 +105,7 @@
 
 SymbolContextList sc_list;
 const size_t matching_symbols = modules.FindSymbolsWithNameAndType (name,
+m_process->GetTarget().GetSymbolRewriter(),
 eSymbolTypeObjCClass,
 sc_list);
 
Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/SymbolRewriter.h"
 #include "lldb/Symbol/Symtab.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
@@ -806,11 +807,14 @@
 }
 
 size_t
-Symtab::FindAllSymbolsWithNameAndType (const ConstString &name, SymbolType symbol_type, std::vector& symbol_indexes)
+Symtab::FindAllSymbolsWithNameAndType (const ConstString &name, const SymbolRewriterSP &rewriter, SymbolType symbol_type, std::vector& symbol_indexes)
 {
 std::lock_guard guard(m_mutex);
 
 Timer scoped_timer (__PRETTY_FUNCTION__, "%s", __PRETTY_FUNCTION__);
+
+ConstString rewrittenName = RewriteName(rewriter, name);
+
 // Initialize

Re: [Lldb-commits] [PATCH] D22286: [LLDB] Help text overhaul

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

Looks good.


http://reviews.llvm.org/D22286



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

Re: [Lldb-commits] [PATCH] D22284: [LLDB] Proposed change in multi-line edit behavior (Return = end/append, Meta+Return = line break)

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We should document this behavior somewhere in some help text, and run 
clang-format.


Repository:
  rL LLVM

http://reviews.llvm.org/D22284



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

[Lldb-commits] [lldb] r275281 - Centralize the way symbol and functions are looked up by making a Module::LookupInfo class that does all of the heavy lifting.

2016-07-13 Thread Greg Clayton via lldb-commits 
Author: gclayton
Date: Wed Jul 13 12:12:24 2016
New Revision: 275281

URL: http://llvm.org/viewvc/llvm-project?rev=275281&view=rev
Log:
Centralize the way symbol and functions are looked up by making a 
Module::LookupInfo class that does all of the heavy lifting.

Background: symbols and functions can be looked up by full mangled name and by 
basename. SymbolFile and ObjectFile are expected to be able to do the lookups 
based on full mangled name or by basename, so when the user types something 
that is incomplete, we must be able to look it up efficiently. For example the 
user types "a::b::c" as a symbol to set a breakpoint on, we will break this 
down into a 'lookup "c"' and then weed out N matches down to just the ones that 
match "a::b::c". Previously this was done manaully in many functions by calling 
Module::PrepareForFunctionNameLookup(...) and then doing the lookup and 
manually pruning the results down afterward with duplicated code. Now all 
places use Module::LookupInfo to do the work in one place.

This allowed me to fix the name lookups to look for "func" with 
eFunctionNameTypeFull as the "name_type_mask", and correctly weed the results:

"func", "func()", "func(int)", "a::func()", "b::func()", and "a::b::func()" 
down to just "func", "func()", "func(int)". Previously we would have set 6 
breakpoints, now we correctly set just 3. This also extends to the expression 
parser when it looks up names for functions it needs to not get multiple 
results so we can call the correct function.

 


Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h?rev=275281&r1=275280&r2=275281&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h Wed Jul 13 
12:12:24 2016
@@ -18,6 +18,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Breakpoint/BreakpointResolver.h"
+#include "lldb/Core/Module.h"
 
 namespace lldb_private {
 
@@ -100,26 +101,7 @@ public:
 protected:
 BreakpointResolverName(const BreakpointResolverName &rhs);
 
-struct LookupInfo
-{
-ConstString name;
-ConstString lookup_name;
-uint32_t name_type_mask; // See FunctionNameType
-bool match_name_after_lookup;
-
-LookupInfo () :
-name(),
-lookup_name(),
-name_type_mask (0),
-match_name_after_lookup (false)
-{
-}
-
-void
-Prune (SymbolContextList &sc_list,
-   size_t start_idx) const;
-};
-std::vector m_lookups;
+std::vector m_lookups;
 ConstString m_class_name;
 RegularExpression m_regex;
 Breakpoint::MatchType m_match_type;

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=275281&r1=275280&r2=275281&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Wed Jul 13 12:12:24 2016
@@ -1094,13 +1094,91 @@ public:
 /// set to true to indicate any matches will need to be checked
 /// to make sure they contain \a name.
 //--
-static void
-PrepareForFunctionNameLookup (const ConstString &name,
-  uint32_t name_type_mask,
-  lldb::LanguageType language,
-  ConstString &lookup_name,
-  uint32_t &lookup_name_type_mask,
-  bool &match_name_after_lookup);
+
+//--
+/// @class LookupInfo Module.h "lldb/Core/Module.h"
+/// @brief A class that encapsulates name lookup information.
+///
+/// Users can type a wide variety of partial names when setting
+/// breakpoints by name or when looking for functions by name.
+/// SymbolVendor and SymbolFile objects are only required to implement
+/// name lookup for function basenames and for fully mangled names.
+/// This means if the user types in a partial name, we must reduce this
+/// to a name lookup that will work with all SymbolFile objects. So we
+/// might reduce a name lookup to look for a basename, and then prune
+/// out any results that don't match.
+///
+/// The "m_name" member variabl

[Lldb-commits] [lldb] r275285 - Remove comment that isn't needed anymore.

2016-07-13 Thread Greg Clayton via lldb-commits 
Author: gclayton
Date: Wed Jul 13 12:25:55 2016
New Revision: 275285

URL: http://llvm.org/viewvc/llvm-project?rev=275285&view=rev
Log:
Remove comment that isn't needed anymore.




Modified:
lldb/trunk/include/lldb/Core/Module.h

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=275285&r1=275284&r2=275285&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Wed Jul 13 12:25:55 2016
@@ -1048,53 +1048,6 @@ public:
 bool
 RemapSourceFile (const char *path, std::string &new_path) const;
 
-//--
-/// Prepare to do a function name lookup.
-///
-/// Looking up functions by name can be a tricky thing. LLDB requires
-/// that accelerator tables contain full names for functions as well
-/// as function basenames which include functions, class methods and
-/// class functions. When the user requests that an action use a
-/// function by name, we are sometimes asked to automatically figure
-/// out what a name could possibly map to. A user might request a
-/// breakpoint be set on "count". If no options are supplied to limit
-/// the scope of where to search for count, we will by default match
-/// any function names named "count", all class and instance methods
-/// named "count" (no matter what the namespace or contained context)
-/// and any selectors named "count". If a user specifies "a::b" we
-/// will search for the basename "b", and then prune the results that
-/// don't match "a::b" (note that "c::a::b" and "d::e::a::b" will
-/// match a query of "a::b".
-///
-/// @param[in] name
-/// The user supplied name to use in the lookup
-///
-/// @param[in] name_type_mask
-/// The mask of bits from lldb::FunctionNameType enumerations
-/// that tell us what kind of name we are looking for.
-///
-/// @param[out] language
-/// If known, the language to use for determining the
-/// lookup_name_type_mask.
-///
-/// @param[out] lookup_name
-/// The actual name that will be used when calling
-/// SymbolVendor::FindFunctions() or Symtab::FindFunctionSymbols()
-///
-/// @param[out] lookup_name_type_mask
-/// The actual name mask that should be used in the calls to
-/// SymbolVendor::FindFunctions() or Symtab::FindFunctionSymbols()
-///
-/// @param[out] match_name_after_lookup
-/// A boolean that indicates if we need to iterate through any
-/// match results obtained from SymbolVendor::FindFunctions() or
-/// Symtab::FindFunctionSymbols() to see if the name contains
-/// \a name. For example if \a name is "a::b", this function will
-/// return a \a lookup_name of "b", with \a match_name_after_lookup
-/// set to true to indicate any matches will need to be checked
-/// to make sure they contain \a name.
-//--
-
 //--
 /// @class LookupInfo Module.h "lldb/Core/Module.h"
 /// @brief A class that encapsulates name lookup information.


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

[Lldb-commits] [lldb] r275287 - Fix a check in the objc trampoline handler

2016-07-13 Thread Stephane Sezer via lldb-commits 
Author: sas
Date: Wed Jul 13 12:34:26 2016
New Revision: 275287

URL: http://llvm.org/viewvc/llvm-project?rev=275287&view=rev
Log:
Fix a check in the objc trampoline handler

Summary:
The function FunctionCaller::WriteFunctionArguments returns false on
errors, so they should check for the false return value.

Change by Walter Erquinigo 

Reviewers: jingham, clayborg

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D22278

Modified:

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp?rev=275287&r1=275286&r2=275287&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
 Wed Jul 13 12:34:26 2016
@@ -818,7 +818,7 @@ AppleObjCTrampolineHandler::SetupDispatc
 // if other threads were calling into here, but actually it isn't because 
we allocate a new args structure for
 // this call by passing args_addr = LLDB_INVALID_ADDRESS...
 
-if (impl_function_caller->WriteFunctionArguments(exe_ctx, args_addr, 
dispatch_values, diagnostics))
+if (!impl_function_caller->WriteFunctionArguments(exe_ctx, args_addr, 
dispatch_values, diagnostics))
 {
 if (log)
 {


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

Re: [Lldb-commits] [PATCH] D22286: [LLDB] Help text overhaul

2016-07-13 Thread Jim Ingham via lldb-commits 
jingham added a subscriber: jingham.
jingham requested changes to this revision.
jingham added a reviewer: jingham.
jingham added a comment.
This revision now requires changes to proceed.

This is great!  Thanks for slogging through these.  There are a few comments 
inline.



Comment at: source/Commands/CommandObjectBreakpoint.cpp:1318
@@ -1317,3 +1317,3 @@
 
-)" "The first command disables all the locations of breakpoint 1, \
+)" "The first command disables all locations matching breakpoint 1, \
 the second re-enables the first location."

Not sure what it means for locations to match a breakpoint.  Maybe "all the 
locations generated by breakpoint 1"?


Comment at: source/Commands/CommandObjectExpression.cpp:225-226
@@ -224,4 +224,4 @@
  "expression",
- "Evaluate an expression in the current program context, 
using user defined variables and variables currently in scope.",
+ "Evaluate an expression on the current thread.  Displays 
any returned value with LLDB's default formatting.",
  nullptr,
  eCommandProcessMustBePaused | eCommandTryTargetAPILock),

"the current thread" seems a little terse.  It is important that that the 
expression is run ON the current thread - because you need to know that that 
thread isn't going to make progress during the expression evaluation, though 
other threads might.  But it is equally important to know that the context for 
lookups is the current frame.


Comment at: source/Commands/CommandObjectTarget.cpp:4342-4343
@@ -4341,4 +4341,4 @@
 "target modules search-paths",
-"A set of commands for operating on debugger 
target image search paths.",
+"Commands for operating on debugger target image 
search paths.",
 "target modules search-paths  
[]")
 {

You were pretty consistent about using "modules" not "images".  Do you want to 
do that here?


Comment at: source/Commands/CommandObjectThread.cpp:1107-1108
@@ -1106,4 +1106,4 @@
 "thread until",
-"Run the current or specified thread until it 
reaches a given line number or address or leaves the current function.",
+"Continue until a line number or address is 
reached by the current or specified thread.  Stops when returning from the 
current function as a safety measure.",
 nullptr,
 eCommandRequiresThread|

I think "when returning" in this context makes it sound like it stops before 
the function returns.  That would be better, but pretty tricky to do.  The 
current command stops AFTER returning from the current function.


Comment at: source/Interpreter/CommandObject.cpp:690-691
@@ -689,4 +689,4 @@
 {
-return "Breakpoint ID's consist major and minor numbers;  the major number 
"
+return "Breakpoint IDs consist major and minor numbers;  the major number "
 "corresponds to the single entity that was created with a 'breakpoint set' 
"
 "command; the minor numbers correspond to all the locations that were 
actually "

Breakpoint IDs consist major and minor numbers

isn't a sentence.  Maybe "are specified by"


Comment at: source/Interpreter/CommandObject.cpp:695
@@ -694,2 +694,3 @@
 "3.14, meaning the 14th location set for the 3rd breakpoint.  You can 
specify "
 "all the locations of a breakpoint by just indicating the major breakpoint 
"
+"number. A valid breakpoint ID consists either of just the major number, "

The "just" on this line doesn't add anything, I'd omit it.


Comment at: source/Interpreter/CommandObject.cpp:708
@@ -707,3 +707,3 @@
 "breakpoint locations under a major breakpoint, you can use the major "
 "breakpoint number followed by '.*', eg. '5.*' means all the locations 
under "
 "breakpoint 5.  You can also indicate a range of breakpoints by using "

Why do we support this, when "5.*" and "5" mean exactly the same thing???


http://reviews.llvm.org/D22286



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

If you can't get this right when debug info is present then I would rather not 
have this patch. We enabled debug info on your test and it will call the 
putchar() in the current library so this test will fail and I question the 
viability of the usefulness of this feature.

Also if you can get that fixed, should the rewrite map actually be associated 
with a target? Shouldn't the rewrite map just be associated with a 
lldb_private::Module instead? It doesn't seem like you would want this at the 
target level. And the command should be "target modules rewrite add 
--clang-rewrite-file rewrite.map". It is ok to store the rewrite map in the 
target, but there should probably be a map of module to SymbolRewriter in the 
target.

In general if we add a member variable to ModuleList that is a 
SymbolRewriterSP, then many of the target->GetImages().FindXXX calls don't need 
a new parameter.

Also see inlined comments.

If this can't coexist with debug info I would rather not have this patch.



Comment at: include/lldb/Core/ModuleList.h:272
@@ -271,2 +271,3 @@
 FindFunctions (const ConstString &name,
+   const lldb::SymbolRewriterSP &rewriter,
uint32_t name_type_mask,

We should add a member variable to ModuleList:

```
lldb::SymbolRewriterSP m_rewriter_sp;
```

Then have target place the lldb::SymbolRewriterSP into it's module list. Then 
this extra parameter shouldn't be needed.


Comment at: include/lldb/Core/ModuleList.h:284
@@ -282,2 +283,3 @@
 FindFunctionSymbols (const ConstString &name,
+ const lldb::SymbolRewriterSP &rewriter,
  uint32_t name_type_mask,

Ditto above comment


Comment at: include/lldb/Core/ModuleList.h:408
@@ -405,2 +407,3 @@
 FindSymbolsWithNameAndType (const ConstString &name,
+const lldb::SymbolRewriterSP &rewriter,
 lldb::SymbolType symbol_type,

Ditto above comment


Comment at: include/lldb/Target/Target.h:1139-1149
@@ -1137,2 +1138,13 @@
+
+lldb::SymbolRewriterSP
+GetSymbolRewriter ()
+{
+if (!m_symbol_rewriter)
+{
+m_symbol_rewriter = std::make_shared();
+}
+
+return m_symbol_rewriter;
+}
 
 //--

Why are we making a lldb::SymbolRewriterSP here? This should return an empty 
lldb::SymbolRewriterSP if no one has called the "target symbols rewrite" 
function. Any callers should check for an empty shared pointer before using it.


Comment at: include/lldb/Target/Target.h:1626
@@ -1613,2 +1625,3 @@
 ModuleList  m_images;   ///< The list of images for this 
process (shared libraries and anything dynamically loaded).
+lldb::SymbolRewriterSP m_symbol_rewriter; ///< Symbol rewriter for the 
target.
 SectionLoadHistory m_section_load_history;

any shared pointers should have a _sp suffix.


Comment at: source/API/SBTarget.cpp:1805-1811
@@ -1804,8 +1804,9 @@
 const bool append = true;
 target_sp->GetImages().FindFunctions (ConstString(name), 
+  
target_sp->GetSymbolRewriter(),
   name_type_mask, 
   symbols_ok,
   inlines_ok,
   append, 
   *sb_sc_list);
 }

This isn't needed if you allow ModuleList to have a "lldb::SymbolRewriterSP 
m_rewriter_sp;" member variable. Remove this.


Comment at: source/API/SBTarget.cpp:1837
@@ -1835,3 +1836,3 @@
 default:
-target_sp->GetImages().FindFunctions(ConstString(name), 
eFunctionNameTypeAny, true, true, true, *sb_sc_list);
+target_sp->GetImages().FindFunctions(ConstString(name), 
target_sp->GetSymbolRewriter(), eFunctionNameTypeAny, true, true, true, 
*sb_sc_list);
 break;

This isn't needed if you allow ModuleList to have a "lldb::SymbolRewriterSP 
m_rewriter_sp;" member variable. Remove this.


Comment at: source/API/SBTarget.cpp:2390-2394
@@ -2388,6 +2389,7 @@
 bool append = true;
 target_sp->GetImages().FindSymbolsWithNameAndType 
(ConstString(name),
+   
target_sp->GetSymbolRewriter(),
symbol_type,
*sb_

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg added a comment.

Adding a feature to the debugger that doesn't work with debug info seems like a 
bad idea.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Jim Ingham via lldb-commits 
jingham added a subscriber: jingham.
jingham added a comment.

This approach seems fragile to me.  I'm supposed to remember that some build 
system set some rewriter file that I then have to supply manually to get the 
view of symbols to match reality?  We really try to avoid having to write notes 
on little pieces of paper and typing them back in to get debugging to work...

This should get recorded somewhere in the module that uses it, and then the 
module reader code would be able to find and apply it automatically.  If this 
is passed to the compiler, it should get written into the DWARF.   But it 
sounds more like it gets passed to then linker.  So maybe we can stick this map 
file in a load command, or symbol that the symbol file reader can fetch.

You might need a command to give search paths for these remap files in case you 
move the around, but that's all that the user should need to do by hand.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg added a comment.

Adrian Prantl  suggested that we modify clang to update the debug info for the 
function. In the test case that was attached, the DWARF currently has a 
"putchar" function which should get a linkage name attached to it with 
"__my_putchar". Then the debug info can be correct and we won't need any 
modifications of the rewrite file stuff.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Stephane Sezer via lldb-commits 
sas added a comment.

@jingham, @clayborg, this is indeed a bit fragile as having to specify your 
rewrite maps manually when debugging is very error-prone. I have a patch that 
fetches the path to the rewrite map from the debug info, I'm waiting for this 
one to go in to upload that other diff (trying to make changes smaller so the 
review is easier). The way I do this is by looking at the flags that were 
passed to the compiler, and get the path to the rewrite map from there 
automatically, which then requires no user interaction for this to work.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22286: [LLDB] Help text overhaul

2016-07-13 Thread Enrico Granata via lldb-commits 
granata.enrico added inline comments.


Comment at: source/Commands/CommandObjectCommands.cpp:2272
@@ -2271,3 +2271,3 @@
 "command script",
-"A set of commands for managing or customizing 
script commands.",
+"Commands for managing custom commands implemented 
by by interpreter scripts.",
 "command script  
[]")

'by by' is one too many 'by's


Comment at: source/Commands/CommandObjectLanguage.cpp:26
@@ -25,3 +25,3 @@
 "language",
-"A set of commands for managing language-specific 
functionality.'.",
+"Commands for interacting with language runtimes.",
 "language   
[]"

Maybe "Commands specific to a given source language"?

We have a notion of language runtime in LLDB, but the 'language' command 
doesn't really change the state of those, it's just a convenient grouping for 
commands that are specifically bound to one language


http://reviews.llvm.org/D22286



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Francis Ricci via lldb-commits 
fjricci added a comment.

@clayborg: As you saw when running the test with debug info enabled, we might 
end up calling the non-rewritten `putchar()`, which is due to the compiler 
emitting debug symbols with the non-rewritten name. The `-g0` option is just a 
workaround until we can fix that.

I suppose Adrian Prantl's idea of modifying the emitted DWARF to add a linkage 
name attached to the function would work. Does that mean we would only add an 
entry for the rewritten symbol when lldb parses the DWARF, and ignore the 
non-rewritten function name?


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg added a comment.

In http://reviews.llvm.org/D22294#483213, @sas wrote:

> @jingham, @clayborg, this is indeed a bit fragile as having to specify your 
> rewrite maps manually when debugging is very error-prone. I have a patch that 
> fetches the path to the rewrite map from the debug info, I'm waiting for this 
> one to go in to upload that other diff (trying to make changes smaller so the 
> review is easier). The way I do this is by looking at the flags that were 
> passed to the compiler, and get the path to the rewrite map from there 
> automatically, which then requires no user interaction for this to work.


If we have the compiler modify the DWARF by adding a linkage name to the 
appropriate DW_TAG_subproggram, then we won't need any rewrite file stuff at 
all.

In http://reviews.llvm.org/D22294#483242, @fjricci wrote:

> @clayborg: As you saw when running the test with debug info enabled, we might 
> end up calling the non-rewritten `putchar()`, which is due to the compiler 
> emitting debug symbols with the non-rewritten name. The `-g0` option is just 
> a workaround until we can fix that.
>
> I suppose Adrian Prantl's idea of modifying the emitted DWARF to add a 
> linkage name attached to the function would work. Does that mean we would 
> only add an entry for the rewritten symbol when lldb parses the DWARF, and 
> ignore the non-rewritten function name?


Basically the DWARF currently looks like:

  0x002e: DW_TAG_subprogram [2] *
   DW_AT_low_pc( 0x )
   DW_AT_high_pc( 0x000e )
   DW_AT_frame_base( rbp )
   DW_AT_name( "putchar" )
   DW_AT_decl_file( 
"/Volumes/work/gclayton/Desktop/symbol_rewriter/main.c" )
   DW_AT_decl_line( 3 )
   DW_AT_prototyped( 0x01 )
   DW_AT_type( {0x007a} ( int ) )
   DW_AT_external( 0x01 )

And we would modify it to emit this:

  0x002e: DW_TAG_subprogram [2] *
   DW_AT_low_pc( 0x )
   DW_AT_high_pc( 0x000e )
   DW_AT_frame_base( rbp )
   DW_AT_name( "putchar" )
   DW_AT_linkage_name( "__my_putchar" ) <<< Added by compiler
   DW_AT_decl_file( 
"/Volumes/work/gclayton/Desktop/symbol_rewriter/main.c" )
   DW_AT_decl_line( 3 )
   DW_AT_prototyped( 0x01 )
   DW_AT_type( {0x007a} ( int ) )
   DW_AT_external( 0x01 )

If we do this, then we don't need any modifications for the rewrite stuff at 
all?


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Stephane Sezer via lldb-commits 
sas added a comment.

In http://reviews.llvm.org/D22294#483250, @clayborg wrote:

> In http://reviews.llvm.org/D22294#483213, @sas wrote:
>
> > @jingham, @clayborg, this is indeed a bit fragile as having to specify your 
> > rewrite maps manually when debugging is very error-prone. I have a patch 
> > that fetches the path to the rewrite map from the debug info, I'm waiting 
> > for this one to go in to upload that other diff (trying to make changes 
> > smaller so the review is easier). The way I do this is by looking at the 
> > flags that were passed to the compiler, and get the path to the rewrite map 
> > from there automatically, which then requires no user interaction for this 
> > to work.
>
>
> If we have the compiler modify the DWARF by adding a linkage name to the 
> appropriate DW_TAG_subproggram, then we won't need any rewrite file stuff at 
> all.
>
> In http://reviews.llvm.org/D22294#483242, @fjricci wrote:
>
> > @clayborg: As you saw when running the test with debug info enabled, we 
> > might end up calling the non-rewritten `putchar()`, which is due to the 
> > compiler emitting debug symbols with the non-rewritten name. The `-g0` 
> > option is just a workaround until we can fix that.
> >
> > I suppose Adrian Prantl's idea of modifying the emitted DWARF to add a 
> > linkage name attached to the function would work. Does that mean we would 
> > only add an entry for the rewritten symbol when lldb parses the DWARF, and 
> > ignore the non-rewritten function name?
>
>
> Basically the DWARF currently looks like:
>
>   0x002e: DW_TAG_subprogram [2] *
>DW_AT_low_pc( 0x )
>DW_AT_high_pc( 0x000e )
>DW_AT_frame_base( rbp )
>DW_AT_name( "putchar" )
>DW_AT_decl_file( 
> "/Volumes/work/gclayton/Desktop/symbol_rewriter/main.c" )
>DW_AT_decl_line( 3 )
>DW_AT_prototyped( 0x01 )
>DW_AT_type( {0x007a} ( int ) )
>DW_AT_external( 0x01 )
>
>
> And we would modify it to emit this:
>
>   0x002e: DW_TAG_subprogram [2] *
>DW_AT_low_pc( 0x )
>DW_AT_high_pc( 0x000e )
>DW_AT_frame_base( rbp )
>DW_AT_name( "putchar" )
>DW_AT_linkage_name( "__my_putchar" ) <<< Added by compiler
>DW_AT_decl_file( 
> "/Volumes/work/gclayton/Desktop/symbol_rewriter/main.c" )
>DW_AT_decl_line( 3 )
>DW_AT_prototyped( 0x01 )
>DW_AT_type( {0x007a} ( int ) )
>DW_AT_external( 0x01 )
>
>
> If we do this, then we don't need any modifications for the rewrite stuff at 
> all?


I think some understanding of the rewrites from the debugger is still required 
because the user will still enter commands like `call putchar('a')` and expect 
the `putchar()` they wrote to be called. That `putchar()` they expect is now 
called `_my_putchar()` and unless we rewrite the symbol they typed, they'll end 
up calling the non-rewritten `putchar()`.

One thing to note is that adding the `DW_AT_linkage_name` entry to 
`DW_TAG_subprogram` will fix the bug that we are working around in the unit 
tests with `-g0`, so that is required regardless.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg added a comment.

> I think some understanding of the rewrites from the debugger is still 
> required because the user will still enter commands like `call putchar('a')` 
> and expect the `putchar()` they wrote to be called. That `putchar()` they 
> expect is now called `_my_putchar()` and unless we rewrite the symbol they 
> typed, they'll end up calling the non-rewritten `putchar()`.


No, the function lookup will find both "putchar" and "_my_putchar" will be 
found in your debug info and map to an address that matches the symbol for 
"_my_putchar". So no rewrite stuff will be needed.

> One thing to note is that adding the `DW_AT_linkage_name` entry to 
> `DW_TAG_subprogram` will fix the bug that we are working around in the unit 
> tests with `-g0`, so that is required regardless.


So the way LLDB resolves functions is the function lookup happens for 
"putchar". If multiple results are found, then we prefer the one from the 
current executable, which means we will use the local "putchar". If we find 
multiple and none are in the current shared library we will note that there are 
duplicates and the expression will fail.

So I vote we fix the DWARF and be done.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg added a comment.

Note that during function lookup, we can find **both** "putchar" and 
"__my_putchar" in the debug info, so you will be able to call both.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Stephane Sezer via lldb-commits 
sas added a comment.

In http://reviews.llvm.org/D22294#483264, @clayborg wrote:

> Note that during function lookup, we can find **both** "putchar" and 
> "__my_putchar" in the debug info, so you will be able to call both.


Correct, unless as you pointed out both symbols are in libraries, and not in 
the main executable.

Our experience using this feature of clang has been that both symbols being in 
libraries is the rule rather than the exception.

See r221548 in llvm which introduces the symbol rewriter pass. This 
functionality is used for things like interposing of libraries, etc, or 
experiments where two versions of the same library are running alongside each 
other. This means that in the general use case, you have both the non-rewritten 
as well as the rewritten library loaded in the process.

FWIW, the current version of lldb is already able to call `_my_putchar`, given 
that this is the name present in the Mach-O symbol table.


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Jim Ingham via lldb-commits 
jingham added a comment.

We always resolve duplicate symbols to the "closest" one to the current frame.  
So if there are two versions of putchar and one is in the library containing 
the current frame, we should find that one.  If we aren't getting that right, 
that's a bug.  So the correct symbol should get called in cases where it makes 
sense.  But if you are in main, and you have two libraries with rewritten 
putchar's it isn't clear which one we should call, and I think it isn't all 
that unexpected that we would just raise an error.  If the duplicate symbols 
are both exported, but there's other linker magic to pick one over the other in 
main, then we'd have to know about that to get the right symbol.  But I think 
that's orthogonal to this discussion.

Jim


http://reviews.llvm.org/D22294



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

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Jim Ingham via lldb-commits 
We always resolve duplicate symbols to the "closest" one to the current frame.  
So if there are two versions of putchar and one is in the library containing 
the current frame, we should find that one.  If we aren't getting that right, 
that's a bug.  So the correct symbol should get called in cases where it makes 
sense.  But if you are in main, and you have two libraries with rewritten 
putchar's it isn't clear which one we should call, and I think it isn't all 
that unexpected that we would just raise an error.  If the duplicate symbols 
are both exported, but there's other linker magic to pick one over the other in 
main, then we'd have to know about that to get the right symbol.  But I think 
that's orthogonal to this discussion.

Jim


> On Jul 13, 2016, at 2:55 PM, Stephane Sezer  wrote:
> 
> sas added a comment.
> 
> In http://reviews.llvm.org/D22294#483264, @clayborg wrote:
> 
>> Note that during function lookup, we can find **both** "putchar" and 
>> "__my_putchar" in the debug info, so you will be able to call both.
> 
> 
> Correct, unless as you pointed out both symbols are in libraries, and not in 
> the main executable.
> 
> Our experience using this feature of clang has been that both symbols being 
> in libraries is the rule rather than the exception.
> 
> See r221548 in llvm which introduces the symbol rewriter pass. This 
> functionality is used for things like interposing of libraries, etc, or 
> experiments where two versions of the same library are running alongside each 
> other. This means that in the general use case, you have both the 
> non-rewritten as well as the rewritten library loaded in the process.
> 
> FWIW, the current version of lldb is already able to call `_my_putchar`, 
> given that this is the name present in the Mach-O symbol table.
> 
> 
> http://reviews.llvm.org/D22294
> 
> 
> 

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

Re: [Lldb-commits] [PATCH] D22284: [LLDB] Proposed change in multi-line edit behavior (Return = end/append, Meta+Return = line break)

2016-07-13 Thread Kate Stone via lldb-commits 
k8stone removed rL LLVM as the repository for this revision.
k8stone updated this revision to Diff 63867.
k8stone added a comment.

clang-format applied.  Greg's recommendation to document the multi-line editing 
behavior in help make sense, but should land after the major help rework in 
progress.


http://reviews.llvm.org/D22284

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

Index: source/Host/common/Editline.cpp
===
--- source/Host/common/Editline.cpp
+++ source/Host/common/Editline.cpp
@@ -657,41 +657,9 @@
 // Establish the new cursor position at the start of a line when inserting a line break
 m_revert_cursor_index = 0;
 
-// Don't perform end of input detection or automatic formatting when pasting
+// Don't perform automatic formatting when pasting
 if (!IsInputPending (m_input_file))
 {
-// If this is the end of the last line, treat this as a potential exit
-if (m_current_line_index == m_input_lines.size() - 1 && new_line_fragment.length() == 0)
-{
-bool end_of_input = true;
-if (m_is_input_complete_callback) 
-{
-SaveEditedLine();
-auto lines = GetInputAsStringList();
-end_of_input = m_is_input_complete_callback (this, lines, m_is_input_complete_callback_baton);
-
-// The completion test is allowed to change the input lines when complete
-if (end_of_input)
-{
-m_input_lines.clear();
-for (unsigned index = 0; index < lines.GetSize(); index++)
-{
-#if LLDB_EDITLINE_USE_WCHAR
-m_input_lines.insert (m_input_lines.end(), m_utf8conv.from_bytes (lines[index]));
-#else
-m_input_lines.insert (m_input_lines.end(), lines[index]);
-#endif
-}
-}
-}
-if (end_of_input)
-{
-fprintf (m_output_file, "\n");
-m_editor_status = EditorStatus::Complete;
-return CC_NEWLINE;
-}
-}
-
 // Apply smart indentation
 if (m_fix_indentation_callback) 
 {
@@ -720,8 +688,50 @@
 }
 
 unsigned char
-Editline::DeleteNextCharCommand (int ch)
+Editline::EndOrAddLineCommand(int ch)
 {
+// Don't perform end of input detection when pasting, always treat this as a line break
+if (IsInputPending(m_input_file))
+{
+return BreakLineCommand(ch);
+}
+
+// Save any edits to this line
+SaveEditedLine();
+
+// If this is the end of the last line, consider whether to add a line instead
+const LineInfoW *info = el_wline(m_editline);
+if (m_current_line_index == m_input_lines.size() - 1 && info->cursor == info->lastchar)
+{
+if (m_is_input_complete_callback)
+{
+auto lines = GetInputAsStringList();
+if (!m_is_input_complete_callback(this, lines, m_is_input_complete_callback_baton))
+{
+return BreakLineCommand(ch);
+}
+
+// The completion test is allowed to change the input lines when complete
+m_input_lines.clear();
+for (unsigned index = 0; index < lines.GetSize(); index++)
+{
+#if LLDB_EDITLINE_USE_WCHAR
+m_input_lines.insert(m_input_lines.end(), m_utf8conv.from_bytes(lines[index]));
+#else
+m_input_lines.insert(m_input_lines.end(), lines[index]);
+#endif
+}
+}
+}
+MoveCursor(CursorLocation::EditingCursor, CursorLocation::BlockEnd);
+fprintf(m_output_file, "\n");
+m_editor_status = EditorStatus::Complete;
+return CC_NEWLINE;
+}
+
+unsigned char
+Editline::DeleteNextCharCommand(int ch)
+{
 LineInfoW * info = const_cast(el_wline (m_editline));
 
 // Just delete the next character normally if possible
@@ -860,8 +870,24 @@
 }
 
 unsigned char
-Editline::FixIndentationCommand (int ch)
+Editline::PreviousHistoryCommand(int ch)
 {
+SaveEditedLine();
+
+return RecallHistory(true);
+}
+
+unsigned char
+Editline::NextHistoryCommand(int ch)
+{
+SaveEditedLine();
+
+return RecallHistory(false);
+}
+
+unsigned char
+Editline::FixIndentationCommand(int ch)
+{
 if (!m_fix_indentation_callback)
 return CC_NORM;
 
@@ -1077,6 +1103,10 @@
 el_wset(m_editline, EL_ADDFN, EditLineConstString("lldb-break-line"), EditLineConstString("Insert a line break"),
 (EditlineCommandCallbackType)(
 [](EditLine *editline, int ch) { return Editline::InstanceFor(editline)->BreakLineCommand(ch); }));
+el_wset(m_editline, EL_ADDFN, EditLineConstString("lldb-end-or-add-line"),
+EditLineConstString("End editing or continue when incomplete"),
+(EditlineCommandCallbackType)(
+[](EditLine *editli

Re: [Lldb-commits] [PATCH] D22294: Add functionality for rewriting symbols

2016-07-13 Thread Greg Clayton via lldb-commits 
clayborg added a comment.

In http://reviews.llvm.org/D22294#483311, @sas wrote:

> In http://reviews.llvm.org/D22294#483264, @clayborg wrote:
>
> > Note that during function lookup, we can find **both** "putchar" and 
> > "__my_putchar" in the debug info, so you will be able to call both.
>
>
> Correct, unless as you pointed out both symbols are in libraries, and not in 
> the main executable.
>
> Our experience using this feature of clang has been that both symbols being 
> in libraries is the rule rather than the exception.
>
> See r221548 in llvm which introduces the symbol rewriter pass. This 
> functionality is used for things like interposing of libraries, etc, or 
> experiments where two versions of the same library are running alongside each 
> other. This means that in the general use case, you have both the 
> non-rewritten as well as the rewritten library loaded in the process.
>
> FWIW, the current version of lldb is already able to call `_my_putchar`, 
> given that this is the name present in the Mach-O symbol table.


And adding the rewriting stuff does nothing to help these duplicate symbols. If 
you are not stopped in a.out, and you try to call putchar, your expression will 
fail with duplicate symbols even with your rewriting stuff. So there is no need 
to add it. Just fix the DWARF.


http://reviews.llvm.org/D22294



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

[Lldb-commits] [lldb] r275336 - Added test for setting breakpoints by basename and fullname.

2016-07-13 Thread Greg Clayton via lldb-commits 
Author: gclayton
Date: Wed Jul 13 17:38:54 2016
New Revision: 275336

URL: http://llvm.org/viewvc/llvm-project?rev=275336&view=rev
Log:
Added test for setting breakpoints by basename and fullname.

 


Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py?rev=275336&r1=275335&r2=275336&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespace.py 
Wed Jul 13 17:38:54 2016
@@ -12,6 +12,73 @@ from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+class NamespaceBreakpointTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_breakpoints_func_auto(self):
+"""Test that we can set breakpoints correctly by basename to find all 
functions whose basename is "func"."""
+self.build()
+
+names = [ "func()", "func(int)", "A::B::func()", "A::func()", 
"A::func(int)"]
+
+# Create a target by the debugger.
+exe = os.path.join(os.getcwd(), "a.out")
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+module_list = lldb.SBFileSpecList()
+module_list.Append(lldb.SBFileSpec(exe, False))
+cu_list = lldb.SBFileSpecList()
+# Set a breakpoint by name "func" which should pick up all functions 
whose basename is "func"
+bp = target.BreakpointCreateByName ("func", 
lldb.eFunctionNameTypeAuto, module_list, cu_list);
+for bp_loc in bp:
+name = bp_loc.GetAddress().GetFunction().GetName()
+self.assertTrue(name in names, "make sure breakpoint locations are 
correct for 'func' with eFunctionNameTypeAuto")
+
+def test_breakpoints_func_full(self):
+"""Test that we can set breakpoints correctly by fullname to find all 
functions whose fully qualified name is "func"
+   (no namespaces)."""
+self.build()
+
+names = [ "func()", "func(int)"]
+
+# Create a target by the debugger.
+exe = os.path.join(os.getcwd(), "a.out")
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+module_list = lldb.SBFileSpecList()
+module_list.Append(lldb.SBFileSpec(exe, False))
+cu_list = lldb.SBFileSpecList()
+
+# Set a breakpoint by name "func" whose fullly qualified named matches 
"func" which 
+# should pick up only functions whose basename is "func" and has no 
containing context
+bp = target.BreakpointCreateByName ("func", 
lldb.eFunctionNameTypeFull, module_list, cu_list);
+for bp_loc in bp:
+name = bp_loc.GetAddress().GetFunction().GetName()
+self.assertTrue(name in names, "make sure breakpoint locations are 
correct for 'func' with eFunctionNameTypeFull")
+
+def test_breakpoints_a_func_full(self):
+"""Test that we can set breakpoints correctly by fullname to find all 
functions whose fully qualified name is "A::func"."""
+self.build()
+
+names = [ "A::func()", "A::func(int)"]
+
+# Create a target by the debugger.
+exe = os.path.join(os.getcwd(), "a.out")
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+module_list = lldb.SBFileSpecList()
+module_list.Append(lldb.SBFileSpec(exe, False))
+cu_list = lldb.SBFileSpecList()
+
+# Set a breakpoint by name "A::func" whose fullly qualified named 
matches "A::func" which 
+# should pick up only functions whose basename is "func" and is 
contained in the "A" namespace
+bp = target.BreakpointCreateByName ("A::func", 
lldb.eFunctionNameTypeFull, module_list, cu_list);
+for bp_loc in bp:
+name = bp_loc.GetAddress().GetFunction().GetName()
+self.assertTrue(name in names, "make sure breakpoint locations are 
correct for 'A::func' with eFunctionNameTypeFull")
+
+
 class NamespaceTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)


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

[Lldb-commits] [PATCH] D22322: [LLDB] Fixes for standalone build

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

This patch include two fixes:

* include CheckAtomic to set HAVE_CXX_ATOMICS64_WITHOUT_LIB properly 
(introduced in r274121)
* hint Clang CMake files for LLVM CMake files location (inctroduced in ~ 
r274176)

Repository:
  rL LLVM

http://reviews.llvm.org/D22322

Files:
  cmake/modules/LLDBStandalone.cmake

Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -58,6 +58,7 @@
   set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
   set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
   set(LLVM_MAIN_INCLUDE_DIR ${INCLUDE_DIR} CACHE PATH "Path to llvm/include")
+  set(LLVM_DIR ${LLVM_OBJ_ROOT}/cmake/modules/CMakeFiles CACHE PATH "Path to 
LLVM build tree")
   set(LLVM_BINARY_DIR ${LLVM_OBJ_ROOT} CACHE PATH "Path to LLVM build tree")
   set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
@@ -85,6 +86,7 @@
 
   include(AddLLVM)
   include(HandleLLVMOptions)
+  include(CheckAtomic)
 
   if (PYTHON_EXECUTABLE STREQUAL "")
 set(Python_ADDITIONAL_VERSIONS 3.5 3.4 3.3 3.2 3.1 3.0 2.7 2.6 2.5)


Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -58,6 +58,7 @@
   set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
   set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
   set(LLVM_MAIN_INCLUDE_DIR ${INCLUDE_DIR} CACHE PATH "Path to llvm/include")
+  set(LLVM_DIR ${LLVM_OBJ_ROOT}/cmake/modules/CMakeFiles CACHE PATH "Path to LLVM build tree")
   set(LLVM_BINARY_DIR ${LLVM_OBJ_ROOT} CACHE PATH "Path to LLVM build tree")
   set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
@@ -85,6 +86,7 @@
 
   include(AddLLVM)
   include(HandleLLVMOptions)
+  include(CheckAtomic)
 
   if (PYTHON_EXECUTABLE STREQUAL "")
 set(Python_ADDITIONAL_VERSIONS 3.5 3.4 3.3 3.2 3.1 3.0 2.7 2.6 2.5)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits