[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk updated this revision to Diff 198987.
kwk added a comment.

- Fix https://reviews.llvm.org/D61737#inline-548136


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737

Files:
  lldb/tools/driver/Options.td


Index: lldb/tools/driver/Options.td
===
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -159,6 +159,10 @@
   Alias,
   HelpText<"Alias for --one-line">,
   Group;
+def: Separate<["-"], "ex">,
+  Alias,
+  HelpText<"Alias for --one-line.">,
+  Group;
 
 def one_line_before_file: Separate<["--", "-"], "one-line-before-file">,
   MetaVarName<"">,


Index: lldb/tools/driver/Options.td
===
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -159,6 +159,10 @@
   Alias,
   HelpText<"Alias for --one-line">,
   Group;
+def: Separate<["-"], "ex">,
+  Alias,
+  HelpText<"Alias for --one-line.">,
+  Group;
 
 def one_line_before_file: Separate<["--", "-"], "one-line-before-file">,
   MetaVarName<"">,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk added a comment.

In D61737#1497202 , @jingham wrote:

> I would rather not clutter up the lldb command driver's options with gdb 
> command flags.  That seems like it will make lldb harder to figure out and 
> reduce our freedom to choose reasonable short names for lldb driver options.


It is good to have this discussion early on and that is exactly why I've picked 
this little example option `-ex` in the first place. I was about to add more 
options/flags/parameters that borrow from GDB. Another reason why I *just* want 
to add more GDB commands instead of introducing a *GDB mode* is because I don't 
want to make a false promise. I'd rather try to bring a lot of commands from 
GDB over and if one is missing, that's fine because it might not be used that 
often. But if we have a switch to the `lldb` binary that means we promise 
something that we don't keep. In essence I rather like a soft-gdb-mode in GDB 
just by *accidentally* allowing for the same commands *and* CLI arguments than 
a hard-gdb-mode.

I would also like to mark the driver parameters in the help to underline that 
they are there for compatibility.

@jingham : what is worse, not having the freedom to re-use a shortcode or 
keeping users away? Surely this is a provocative question but my point is that 
the whole point is to make LLDB more appealing as an option for an alternative 
debugger to GDB.

> It was reasonable to add lldb aliases for the gdb commands that you use tens 
> to hundreds of times in a give debugging session - those get wired into your 
> hands...  But I don't think the same consideration holds for command line 
> options...

Well, true the commands in a debugging session are harder to remember and I 
will for sure tackle commands in the future but from a user's or IDE's point of 
view, starting the debugger comes first. Especially the IDE view on things 
might be worth considering here. Users can adapt but probably not all Debuggers 
inside of IDEs can that easily.

> If we feel the need to add a driver gdb compatibility mode like this, I like 
> Rafael's suggestion of:
> 
> lldb --gdb 

This makes things much more complicated than it needs to be and puts us in a 
bad position because we then always promise to keep up with GDBs commands 
somehow. As I've mentioned before, I rather want to accidentally *bridge* the 
ease of adoption. Let's not put too much hurdles in a user's way just in order 
to get going with lldb.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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


[Lldb-commits] [PATCH] D61732: FuncUnwinders: Add a new "SymbolFile" unwind plan

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D61732#1497609 , @jasonmolenda 
wrote:

> I'm not super concerned about the size of the UnwindPlan or FuncUnwinders 
> objects - we create them lazily as we unwind through functions in the 
> process, so my guess is that even a long-running debug session will have on 
> the order of thousands of these objects.  The ABIs don't even bother to issue 
> a single ArchDefault unwind plan and FunctionEntry unwind plan, handing out a 
> new one for each FuncUnwinders object (ok that part is a little embarrassing, 
> but again this isn't super high on the priority list to fix imo.)


Ok, sounds good. I'll whip up a patch to merge the _sp and _tried fields into 
one entity.


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

https://reviews.llvm.org/D61732



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


[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Eric Christopher via Phabricator via lldb-commits
echristo added a comment.

In D61737#1497202 , @jingham wrote:

> I would rather not clutter up the lldb command driver's options with gdb 
> command flags.  That seems like it will make lldb harder to figure out and 
> reduce our freedom to choose reasonable short names for lldb driver options.
>
> It was reasonable to add lldb aliases for the gdb commands that you use tens 
> to hundreds of times in a give debugging session - those get wired into your 
> hands...  But I don't think the same consideration holds for command line 
> options...
>
> If we feel the need to add a driver gdb compatibility mode like this, I like 
> Rafael's suggestion of:
>
> lldb --gdb 


Peanut gallery comments :)

Since I've had to deal with some of this from the gnu binutils direction as 
well. I think if we really want to have a command-line compatibility mode with 
lldb we should just have a gdb wrapper/shell/parsed option on top of the lldb 
libraries/binary. Some of this would necessitate better library splitting of 
lldb to really have a gdb compatible shell on top but it could be done. This 
isn't quite how we've done it so far in llvm's binutils, but as an aspirational 
strategy I think we can and should make it work in general. It'll also help 
drive much cleaner APIs as well since we'll have to implement a few things from 
different perspectives.

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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


[Lldb-commits] [lldb] r360409 - FuncUnwinders: Add a new "SymbolFile" unwind plan

2019-05-10 Thread Pavel Labath via lldb-commits
Author: labath
Date: Fri May 10 00:54:37 2019
New Revision: 360409

URL: http://llvm.org/viewvc/llvm-project?rev=360409&view=rev
Log:
FuncUnwinders: Add a new "SymbolFile" unwind plan

Summary:
some unwind formats are specific to a single symbol file and so it does
not make sense for their parsing code live in the general Symbol library
(as is the case with eh_frame for instance). This is the case for the
unwind information in breakpad files, but the same will probably be true
for PDB unwind info (once we are able to parse that).

This patch adds the ability to fetch an unwind plan provided by a symbol
file plugin, as discussed in the RFC at
.
I've kept the set of changes to a minimum, as there is no way to test
them until we have a symbol file which implements this API -- that is
comming in a follow-up patch, which will also implicitly test this
change.

The interesting part here is the introduction of the
"RegisterInfoResolver" interface. The reason for this is that breakpad
needs to be able to resolve register names (which are present as strings
in the file) into register enums so that it can construct the unwind
plan. This is normally done via the RegisterContext class, handing this
over to the SymbolFile plugin would mean that it has full access to the
debugged process, which is not something we want it to have. So instead,
I create a facade, which only provides the ability to query register
names, and hide the RegisterContext behind the facade.

Also note that this only adds the ability to dump the unwind plan
created by the symbol file plugin -- the plan is not used for unwinding
yet -- this will be added in a third patch, which will add additional
tests which makes sure the unwinding works as a whole.

Reviewers: jasonmolenda, clayborg

Subscribers: markmentovai, amccarth, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Symbol/FuncUnwinders.h
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/UnwindTable.h
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Symbol/FuncUnwinders.cpp
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/source/Symbol/UnwindTable.cpp

Modified: lldb/trunk/include/lldb/Symbol/FuncUnwinders.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/FuncUnwinders.h?rev=360409&r1=360408&r2=360409&view=diff
==
--- lldb/trunk/include/lldb/Symbol/FuncUnwinders.h (original)
+++ lldb/trunk/include/lldb/Symbol/FuncUnwinders.h Fri May 10 00:54:37 2019
@@ -90,6 +90,8 @@ public:
 
   lldb::UnwindPlanSP GetArmUnwindUnwindPlan(Target &target);
 
+  lldb::UnwindPlanSP GetSymbolFileUnwindPlan(Thread &thread);
+
   lldb::UnwindPlanSP GetArchDefaultUnwindPlan(Thread &thread);
 
   lldb::UnwindPlanSP GetArchDefaultAtFuncEntryUnwindPlan(Thread &thread);
@@ -120,6 +122,7 @@ private:
 
   std::vector m_unwind_plan_compact_unwind;
   lldb::UnwindPlanSP m_unwind_plan_arm_unwind_sp;
+  lldb::UnwindPlanSP m_unwind_plan_symbol_file_sp;
   lldb::UnwindPlanSP m_unwind_plan_fast_sp;
   lldb::UnwindPlanSP m_unwind_plan_arch_default_sp;
   lldb::UnwindPlanSP m_unwind_plan_arch_default_at_func_entry_sp;
@@ -131,8 +134,8 @@ private:
   m_tried_unwind_plan_eh_frame_augmented : 1,
   m_tried_unwind_plan_debug_frame_augmented : 1,
   m_tried_unwind_plan_compact_unwind : 1,
-  m_tried_unwind_plan_arm_unwind : 1, m_tried_unwind_fast : 1,
-  m_tried_unwind_arch_default : 1,
+  m_tried_unwind_plan_arm_unwind : 1, m_tried_unwind_plan_symbol_file : 1,
+  m_tried_unwind_fast : 1, m_tried_unwind_arch_default : 1,
   m_tried_unwind_arch_default_at_func_entry : 1;
 
   Address m_first_non_prologue_insn;

Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=360409&r1=360408&r2=360409&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Fri May 10 00:54:37 2019
@@ -223,6 +223,18 @@ public:
   /// for this module have been changed.
   virtual void SectionFileAddressesChanged() {}
 
+  struct RegisterInfoResolver {
+virtual ~RegisterInfoResolver(); // anchor
+
+virtual const RegisterInfo *ResolveName(llvm::StringRef name) const = 0;
+virtual const RegisterInfo *ResolveNumber(lldb::RegisterKind kind,
+  uint32_t number) const = 0;
+  };
+  virtual lldb::UnwindPlanSP
+  GetUnwindPlan(const Address &address, const RegisterInfoResolver &resolver) {
+return nullptr;
+  }
+
   virtual void Dump(Stream &s) {}
 
 protected:

Modified: lldb/trunk/include/lldb/Symbol/UnwindTable.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk

[Lldb-commits] [PATCH] D61732: FuncUnwinders: Add a new "SymbolFile" unwind plan

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB360409: FuncUnwinders: Add a new "SymbolFile" 
unwind plan (authored by labath, committed by ).
Herald added a subscriber: abidh.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D61732?vs=198817&id=198988#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61732

Files:
  include/lldb/Symbol/FuncUnwinders.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/UnwindTable.h
  source/Commands/CommandObjectTarget.cpp
  source/Symbol/FuncUnwinders.cpp
  source/Symbol/SymbolFile.cpp
  source/Symbol/UnwindTable.cpp

Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -13,11 +13,13 @@
 #include "lldb/Symbol/CompactUnwindInfo.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/UnwindPlan.h"
 #include "lldb/Symbol/UnwindTable.h"
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/RegisterNumber.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
@@ -42,7 +44,8 @@
   m_tried_unwind_plan_eh_frame_augmented(false),
   m_tried_unwind_plan_debug_frame_augmented(false),
   m_tried_unwind_plan_compact_unwind(false),
-  m_tried_unwind_plan_arm_unwind(false), m_tried_unwind_fast(false),
+  m_tried_unwind_plan_arm_unwind(false),
+  m_tried_unwind_plan_symbol_file(false), m_tried_unwind_fast(false),
   m_tried_unwind_arch_default(false),
   m_tried_unwind_arch_default_at_func_entry(false),
   m_first_non_prologue_insn() {}
@@ -147,6 +150,38 @@
   return m_unwind_plan_arm_unwind_sp;
 }
 
+namespace {
+class RegisterContextToInfo: public SymbolFile::RegisterInfoResolver {
+public:
+  RegisterContextToInfo(RegisterContext &ctx) : m_ctx(ctx) {}
+
+  const RegisterInfo *ResolveName(llvm::StringRef name) const {
+return m_ctx.GetRegisterInfoByName(name);
+  }
+  const RegisterInfo *ResolveNumber(lldb::RegisterKind kind,
+uint32_t number) const {
+return m_ctx.GetRegisterInfo(kind, number);
+  }
+
+private:
+  RegisterContext &m_ctx;
+};
+} // namespace
+
+UnwindPlanSP FuncUnwinders::GetSymbolFileUnwindPlan(Thread &thread) {
+  std::lock_guard guard(m_mutex);
+  if (m_unwind_plan_symbol_file_sp.get() || m_tried_unwind_plan_symbol_file)
+return m_unwind_plan_symbol_file_sp;
+
+  m_tried_unwind_plan_symbol_file = true;
+  if (SymbolFile *symfile = m_unwind_table.GetSymbolFile()) {
+m_unwind_plan_symbol_file_sp = symfile->GetUnwindPlan(
+m_range.GetBaseAddress(),
+RegisterContextToInfo(*thread.GetRegisterContext()));
+  }
+  return m_unwind_plan_symbol_file_sp;
+}
+
 UnwindPlanSP FuncUnwinders::GetEHFrameAugmentedUnwindPlan(Target &target,
   Thread &thread) {
   std::lock_guard guard(m_mutex);
Index: source/Symbol/UnwindTable.cpp
===
--- source/Symbol/UnwindTable.cpp
+++ source/Symbol/UnwindTable.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Symbol/FuncUnwinders.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/SymbolVendor.h"
 
 // There is one UnwindTable object per ObjectFile. It contains a list of Unwind
 // objects -- one per function, populated lazily -- for the ObjectFile. Each
@@ -181,6 +182,12 @@
   return m_arm_unwind_up.get();
 }
 
+SymbolFile *UnwindTable::GetSymbolFile() {
+  if (SymbolVendor *vendor = m_module.GetSymbolVendor())
+return vendor->GetSymbolFile();
+  return nullptr;
+}
+
 ArchSpec UnwindTable::GetArchitecture() { return m_module.GetArchitecture(); }
 
 bool UnwindTable::GetAllowAssemblyEmulationUnwindPlans() {
Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -168,3 +168,5 @@
  "Module is not locked");
 #endif
 }
+
+SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
Index: source/Commands/CommandObjectTarget.cpp
===
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -3591,6 +3591,14 @@
 result.GetOutputStream().Printf("\n");
   }
 
+  if (UnwindPlanSP symfile_plan_sp =
+  func_unwinders_sp->GetSymbolFileUnwindPlan(*thread)) {
+result.GetOutputStream().Printf("Symbol file UnwindPlan:\n");
+symfile_plan_sp->Dump(result.GetOutputStream(), thread.get(),
+  LLDB_INVALID_

[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

In D61737#1497785 , @kwk wrote:

> In D61737#1497202 , @jingham wrote:
>
> > I would rather not clutter up the lldb command driver's options with gdb 
> > command flags.  That seems like it will make lldb harder to figure out and 
> > reduce our freedom to choose reasonable short names for lldb driver options.
>
>
> It is good to have this discussion early on and that is exactly why I've 
> picked this little example option `-ex` in the first place. I was about to 
> add more options/flags/parameters that borrow from GDB. Another reason why I 
> *just* want to add more GDB commands instead of introducing a *GDB mode* is 
> because I don't want to make a false promise. I'd rather try to bring a lot 
> of commands from GDB over and if one is missing, that's fine because it might 
> not be used that often. But if we have a switch to the `lldb` binary that 
> means we promise something that we don't keep. In essence I rather like a 
> soft-gdb-mode in GDB just by *accidentally* allowing for the same commands 
> *and* CLI arguments than a hard-gdb-mode.


I agree, but that doesn't solve the problem with e.g. `-x` that is already 
occupied by LLDB but with a different meaning that in GDB. If we add a bunch of 
GDB options we also create the expectation that LLDB just accepts GDB flags 
which is as discussed not really possible. I think some form of 
gdb-mode/wrapper is the cleanest option for compatibility with tooling and 
advanced users (and it seems to be the consensus in this review).

And for just making the first time lldb user experience smoother, I think 
adding `--args` seems like a good compromise. It's probably the first flag 
people throw at GDB and LLDB and doesn't seem too out of place when we add it.

> I would also like to mark the driver parameters in the help to underline that 
> they are there for compatibility.

Yeah, If we land this I would also put them in some option group "GDB 
Compatibility" like the REPL or SCRIPTING groups we already have.

> @jingham : what is worse, not having the freedom to re-use a shortcode or 
> keeping users away? Surely this is a provocative question but my point is 
> that the whole point is to make LLDB more appealing as an option for an 
> alternative debugger to GDB.
> 
>> It was reasonable to add lldb aliases for the gdb commands that you use tens 
>> to hundreds of times in a give debugging session - those get wired into your 
>> hands...  But I don't think the same consideration holds for command line 
>> options...
> 
> Well, true the commands in a debugging session are harder to remember and I 
> will for sure tackle commands in the future but from a user's or IDE's point 
> of view, starting the debugger comes first. Especially the IDE view on things 
> might be worth considering here. Users can adapt but probably not all 
> Debuggers inside of IDEs can that easily.

I don't think IDE's are supposed to invoke the normal lldb driver (see lldb-mi 
or the SB* API). And if they use the normal lldb CLI, then they probably expect 
more GDB similarities like identical commands, identical stdout output format, 
etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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


[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: jasonmolenda, clayborg.

A large chunk of this file was dealing with the caching of unwind plans.
In this patch I create a helper class to encapsulate this behavior and
leave the functions to deal with the actual work, which is to compute
the actual plan.

Since the caching now is handled by a separate class, it is also
possible to optimize it better without hampering readability. The way I
achieve that is by using a special shared_ptr value to mean "not
initialized" which means we can avoid having a special bool flag for
that purpose. This results in a net decrease of the size of the
FuncUnwinders object.


https://reviews.llvm.org/D61779

Files:
  include/lldb/Symbol/FuncUnwinders.h
  source/Symbol/FuncUnwinders.cpp

Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -30,25 +30,25 @@
 using namespace lldb;
 using namespace lldb_private;
 
+/// Return a special value meaning the unwind plan hasn't been computed yet.
+static lldb::UnwindPlanSP GetUncomputedMarkerValue() {
+  static auto value_sp = std::make_shared(eRegisterKindGeneric);
+  return value_sp;
+}
+
+FuncUnwinders::LazyPlan::LazyPlan() : m_plan_sp(GetUncomputedMarkerValue()) {}
+
+lldb::UnwindPlanSP
+FuncUnwinders::LazyPlan::Get(llvm::function_ref compute) {
+  if (m_plan_sp == GetUncomputedMarkerValue())
+m_plan_sp = compute();
+  return m_plan_sp;
+}
+
 /// constructor
 
 FuncUnwinders::FuncUnwinders(UnwindTable &unwind_table, AddressRange range)
-: m_unwind_table(unwind_table), m_range(range), m_mutex(),
-  m_unwind_plan_assembly_sp(), m_unwind_plan_eh_frame_sp(),
-  m_unwind_plan_eh_frame_augmented_sp(), m_unwind_plan_compact_unwind(),
-  m_unwind_plan_arm_unwind_sp(), m_unwind_plan_fast_sp(),
-  m_unwind_plan_arch_default_sp(),
-  m_unwind_plan_arch_default_at_func_entry_sp(),
-  m_tried_unwind_plan_assembly(false), m_tried_unwind_plan_eh_frame(false),
-  m_tried_unwind_plan_debug_frame(false),
-  m_tried_unwind_plan_eh_frame_augmented(false),
-  m_tried_unwind_plan_debug_frame_augmented(false),
-  m_tried_unwind_plan_compact_unwind(false),
-  m_tried_unwind_plan_arm_unwind(false),
-  m_tried_unwind_plan_symbol_file(false), m_tried_unwind_fast(false),
-  m_tried_unwind_arch_default(false),
-  m_tried_unwind_arch_default_at_func_entry(false),
-  m_first_non_prologue_insn() {}
+: m_unwind_table(unwind_table), m_range(range) {}
 
 /// destructor
 
@@ -71,83 +71,66 @@
 
 UnwindPlanSP FuncUnwinders::GetCompactUnwindUnwindPlan(Target &target) {
   std::lock_guard guard(m_mutex);
-  if (m_unwind_plan_compact_unwind.size() > 0)
-return m_unwind_plan_compact_unwind[0]; // FIXME support multiple compact
-// unwind plans for one func
-  if (m_tried_unwind_plan_compact_unwind)
-return UnwindPlanSP();
 
-  m_tried_unwind_plan_compact_unwind = true;
-  if (m_range.GetBaseAddress().IsValid()) {
-Address current_pc(m_range.GetBaseAddress());
-CompactUnwindInfo *compact_unwind = m_unwind_table.GetCompactUnwindInfo();
-if (compact_unwind) {
-  UnwindPlanSP unwind_plan_sp(new UnwindPlan(lldb::eRegisterKindGeneric));
-  if (compact_unwind->GetUnwindPlan(target, current_pc, *unwind_plan_sp)) {
-m_unwind_plan_compact_unwind.push_back(unwind_plan_sp);
-return m_unwind_plan_compact_unwind[0]; // FIXME support multiple
-// compact unwind plans for one
-// func
+  // FIXME support multiple compact unwind plans for one func
+  return m_unwind_plan_compact_unwind.Get([&] {
+if (m_range.GetBaseAddress().IsValid()) {
+  Address current_pc(m_range.GetBaseAddress());
+  CompactUnwindInfo *compact_unwind = m_unwind_table.GetCompactUnwindInfo();
+  if (compact_unwind) {
+UnwindPlanSP unwind_plan_sp(new UnwindPlan(lldb::eRegisterKindGeneric));
+if (compact_unwind->GetUnwindPlan(target, current_pc,
+  *unwind_plan_sp)) {
+  return unwind_plan_sp;
+}
   }
 }
-  }
-  return UnwindPlanSP();
+return UnwindPlanSP();
+  });
 }
 
 UnwindPlanSP FuncUnwinders::GetEHFrameUnwindPlan(Target &target) {
   std::lock_guard guard(m_mutex);
-  if (m_unwind_plan_eh_frame_sp.get() || m_tried_unwind_plan_eh_frame)
-return m_unwind_plan_eh_frame_sp;
-
-  m_tried_unwind_plan_eh_frame = true;
-  if (m_range.GetBaseAddress().IsValid()) {
-DWARFCallFrameInfo *eh_frame = m_unwind_table.GetEHFrameInfo();
-if (eh_frame) {
-  m_unwind_plan_eh_frame_sp =
-  std::make_shared(lldb::eRegisterKindGeneric);
-  if (!eh_frame->GetUnwindPlan(m_range, *m_unwind_plan_eh_frame_sp))
-m_unwind_plan_eh_frame_sp.reset();
+  return m_unwind_plan_eh_frame

Re: [Lldb-commits] Fwd: buildbot failure in LLVM on lldb-x64-windows-ninja

2019-05-10 Thread Pavel Labath via lldb-commits

On 10/05/2019 00:20, Greg Clayton via lldb-commits wrote:
Can anyone that builds the windows lldb compile the following file as 
the test suite would:


trunk/packages/Python/lldbsuite/test/python_api/thread/main2.cpp

and send me the binary that is produced by the test suite?

I see the windows bot is failing and need to see binary that we produce 
so I can disassemble and see what is going wrong.


Greg



I am pretty sure your commit actually "fixed" step over in this test on 
windows. The bot was red because this test was now unexpectedly passing.


What I happened was that due to bad unwinding support on windows, the 
lldb would previously step into some function, and then couldn't figure 
out where it is, so it aborted the step over. Now, it no longer attempts 
to step into the function, and so the step over completes successfully.


I've reverted r360397 and removed the windows XFAILs in 360407.

BTW: I think r360397 was a pretty bad way to "revert" a patch. If you 
think your commit has broken something, you should just revert the whole 
thing.

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


[Lldb-commits] [PATCH] D61733: Breakpad: Generate unwind plans from STACK CFI records

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath marked 3 inline comments as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:472
+  if (!unwind_rules.empty())
+LLDB_LOG(log, "Could not parse `{0}` as an unwind rule.", unwind_rules);
+  return true;

amccarth wrote:
> Should the function return `false` when there are leftover unparsable rules?  
> The other error cases seem to do that.
Good point. In fact this should have been caught by the test, if it was written 
properly, but I forgot the colon after the `CHECK-NOT` directive, which meant 
it was ignored by FileCheck. :/
Fixing that found another case where I was mistakenly returning a "valid" 
unwind plan for invalid input, which I have fixed too.


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

https://reviews.llvm.org/D61733



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


[Lldb-commits] [PATCH] D61733: Breakpad: Generate unwind plans from STACK CFI records

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 198993.
labath added a comment.

- use a yaml form of the minidump file (as it is possible to do that as of 
today)
- fix the CHECK-NOT directives in the test and the two issues they caught
- found a way to trigger one of the asserts with an invalid input, so I demote 
it into a "return nullptr"


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

https://reviews.llvm.org/D61733

Files:
  lit/SymbolFile/Breakpad/Inputs/stack-cfi-parsing.syms
  lit/SymbolFile/Breakpad/Inputs/stack-cfi-parsing.yaml
  lit/SymbolFile/Breakpad/stack-cfi-parsing.test
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h

Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
@@ -13,6 +13,7 @@
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/SymbolFile.h"
+#include "lldb/Symbol/UnwindPlan.h"
 
 namespace lldb_private {
 
@@ -133,6 +134,10 @@
 
   void AddSymbols(Symtab &symtab) override;
 
+  lldb::UnwindPlanSP
+  GetUnwindPlan(const Address &address,
+const RegisterInfoResolver &resolver) override;
+
   ConstString GetPluginName() override { return GetPluginNameStatic(); }
   uint32_t GetPluginVersion() override { return 1; }
 
@@ -144,6 +149,11 @@
   struct Bookmark {
 uint32_t section;
 size_t offset;
+
+friend bool operator<(const Bookmark &lhs, const Bookmark &rhs) {
+  return std::tie(lhs.section, lhs.offset) <
+ std::tie(rhs.section, rhs.offset);
+}
   };
 
   // At iterator class for simplifying algorithms reading data from the breakpad
@@ -177,8 +187,7 @@
   return *this;
 }
 friend bool operator<(const CompUnitData &lhs, const CompUnitData &rhs) {
-  return std::tie(lhs.bookmark.section, lhs.bookmark.offset) <
- std::tie(rhs.bookmark.section, rhs.bookmark.offset);
+  return lhs.bookmark < rhs.bookmark;
 }
 
 Bookmark bookmark;
@@ -192,11 +201,19 @@
   void ParseFileRecords();
   void ParseCUData();
   void ParseLineTableAndSupportFiles(CompileUnit &cu, CompUnitData &data);
+  void ParseUnwindData();
+  bool ParseUnwindRow(llvm::StringRef unwind_rules,
+  const RegisterInfoResolver &resolver,
+  UnwindPlan::Row &row);
 
   using CompUnitMap = RangeDataVector;
 
   llvm::Optional> m_files;
   llvm::Optional m_cu_data;
+
+  using UnwindMap = RangeDataVector;
+  llvm::Optional m_unwind_data;
+  llvm::BumpPtrAllocator m_allocator;
 };
 
 } // namespace breakpad
Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -15,9 +15,11 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/PostfixExpression.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/StringExtras.h"
 
 using namespace lldb;
@@ -370,6 +372,155 @@
   symtab.CalculateSymbolSizes();
 }
 
+static llvm::Optional>
+GetRule(llvm::StringRef &unwind_rules) {
+  // Unwind rules are of the form
+  //   register1: expression1 register2: expression2 ...
+  // We assume none of the tokens in expression end with a colon.
+
+  llvm::StringRef lhs, rest;
+  std::tie(lhs, rest) = getToken(unwind_rules);
+  if (!lhs.consume_back(":"))
+return llvm::None;
+
+  // Seek forward to the next register: expression pair
+  llvm::StringRef::size_type pos = rest.find(": ");
+  if (pos == llvm::StringRef::npos) {
+// No pair found, this means the rest of the string is a single expression.
+unwind_rules = llvm::StringRef();
+return std::make_pair(lhs, rest);
+  }
+
+  // Go back one token to find the end of the current rule.
+  pos = rest.rfind(' ', pos);
+  if (pos == llvm::StringRef::npos)
+return llvm::None;
+
+  llvm::StringRef rhs = rest.take_front(pos);
+  unwind_rules = rest.drop_front(pos);
+  return std::make_pair(lhs, rhs);
+}
+
+static const RegisterInfo *
+ResolveRegister(const SymbolFile::RegisterInfoResolver &resolver,
+llvm::StringRef name) {
+  if (name.consume_front("$"))
+return resolver.ResolveName(name);
+
+  return nullptr;
+}
+
+static const RegisterInfo *
+ResolveRegisterOrRA(const SymbolFile::RegisterInfoResolver &resolver,
+llvm::StringRef name) {
+  if (name == ".ra")
+return resolver.ResolveNumber(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC);
+  return ResolveRegister(resolver, name);
+}
+
+bool SymbolFileBreakpad::ParseUnwindRo

[Lldb-commits] [lldb] r360412 - Minidump: use ThreadList parsing code from llvm/Object

2019-05-10 Thread Pavel Labath via lldb-commits
Author: labath
Date: Fri May 10 02:36:11 2019
New Revision: 360412

URL: http://llvm.org/viewvc/llvm-project?rev=360412&view=rev
Log:
Minidump: use ThreadList parsing code from llvm/Object

Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h
lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.cpp
lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.h
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=360412&r1=360411&r2=360412&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Fri May 10 
02:36:11 2019
@@ -89,13 +89,15 @@ UUID MinidumpParser::GetModuleUUID(const
   return UUID();
 }
 
-llvm::ArrayRef MinidumpParser::GetThreads() {
-  llvm::ArrayRef data = GetStream(StreamType::ThreadList);
-
-  if (data.size() == 0)
-return llvm::None;
-
-  return MinidumpThread::ParseThreadList(data);
+llvm::ArrayRef MinidumpParser::GetThreads() {
+  auto ExpectedThreads = GetMinidumpFile().getThreadList();
+  if (ExpectedThreads)
+return *ExpectedThreads;
+
+  LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD),
+ ExpectedThreads.takeError(),
+ "Failed to read thread list: {0}");
+  return {};
 }
 
 llvm::ArrayRef
@@ -106,19 +108,19 @@ MinidumpParser::GetThreadContext(const L
 }
 
 llvm::ArrayRef
-MinidumpParser::GetThreadContext(const MinidumpThread &td) {
-  return GetThreadContext(td.thread_context);
+MinidumpParser::GetThreadContext(const minidump::Thread &td) {
+  return GetThreadContext(td.Context);
 }
 
 llvm::ArrayRef
-MinidumpParser::GetThreadContextWow64(const MinidumpThread &td) {
+MinidumpParser::GetThreadContextWow64(const minidump::Thread &td) {
   // On Windows, a 32-bit process can run on a 64-bit machine under WOW64. If
   // the minidump was captured with a 64-bit debugger, then the CONTEXT we just
   // grabbed from the mini_dump_thread is the one for the 64-bit "native"
   // process rather than the 32-bit "guest" process we care about.  In this
   // case, we can get the 32-bit CONTEXT from the TEB (Thread Environment
   // Block) of the 64-bit process.
-  auto teb_mem = GetMemory(td.teb, sizeof(TEB64));
+  auto teb_mem = GetMemory(td.EnvironmentBlock, sizeof(TEB64));
   if (teb_mem.empty())
 return {};
 
@@ -329,15 +331,14 @@ MinidumpParser::FindMemoryRange(lldb::ad
 return llvm::None;
 
   if (!data.empty()) {
-llvm::ArrayRef memory_list =
-MinidumpMemoryDescriptor::ParseMemoryList(data);
+llvm::ArrayRef memory_list = ParseMemoryList(data);
 
 if (memory_list.empty())
   return llvm::None;
 
 for (const auto &memory_desc : memory_list) {
-  const LocationDescriptor &loc_desc = memory_desc.memory;
-  const lldb::addr_t range_start = memory_desc.start_of_memory_range;
+  const LocationDescriptor &loc_desc = memory_desc.Memory;
+  const lldb::addr_t range_start = memory_desc.StartOfMemoryRange;
   const size_t range_size = loc_desc.DataSize;
 
   if (loc_desc.RVA + loc_desc.DataSize > GetData().size())
@@ -452,16 +453,16 @@ CreateRegionsCacheFromMemoryList(Minidum
   auto data = parser.GetStream(StreamType::MemoryList);
   if (data.empty())
 return false;
-  auto memory_list = MinidumpMemoryDescriptor::ParseMemoryList(data);
+  auto memory_list = ParseMemoryList(data);
   if (memory_list.empty())
 return false;
   regions.reserve(memory_list.size());
   for (const auto &memory_desc : memory_list) {
-if (memory_desc.memory.DataSize == 0)
+if (memory_desc.Memory.DataSize == 0)
   continue;
 MemoryRegionInfo region;
-region.GetRange().SetRangeBase(memory_desc.start_of_memory_range);
-region.GetRange().SetByteSize(memory_desc.memory.DataSize);
+region.GetRange().SetRangeBase(memory_desc.StartOfMemoryRange);
+region.GetRange().SetByteSize(memory_desc.Memory.DataSize);
 region.SetReadable(MemoryRegionInfo::eYes);
 region.SetMapped(MemoryRegionInfo::eYes);
 regions.push_back(region);

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h?rev=360412&r1=360411&r2=360412&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/Minid

[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk added a comment.

@teemperor Do you recommend me to work on support for GDB's `--args` parameter 
then and ditch this review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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


[Lldb-commits] [PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

2019-05-10 Thread Gabor Marton via Phabricator via lldb-commits
martong marked an inline comment as done.
martong added inline comments.



Comment at: clang/include/clang/AST/ASTImporter.h:203
 /// context, or the import error.
-llvm::Expected Import_New(TypeSourceInfo *FromTSI);
-// FIXME: Remove this version.
-TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
+llvm::Expected Import(TypeSourceInfo *FromTSI);
 

aprantl wrote:
> Wouldn't it make more sense to return `Expected`?
That would not be consistent with the other changes. In this patch we change 
the signature of each functions similarly:
From `T foo(...)` to `Expected foo(...)`.
Also, `TypeSourceInfo` factory functions in `ASTContext` do return with a 
pointer. For example:
```
  TypeSourceInfo *CreateTypeSourceInfo(QualType T, unsigned Size = 0) const;

  /// Allocate a TypeSourceInfo where all locations have been
  /// initialized to a given location, which defaults to the empty
  /// location.
  TypeSourceInfo *
  getTrivialTypeSourceInfo(QualType T,
   SourceLocation Loc = SourceLocation()) const;

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61438



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


[Lldb-commits] [PATCH] D61783: [DWARF] Use sequential integers for the IDs of the SymbolFileDWOs

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: JDevlieghere, clayborg, aprantl.
Herald added subscribers: dexonsmith, mehdi_amini.

Instead of using the offset of the contained compile unit, we use it's
ID. The goal of this change is two-fold:

- free up space in the user_id_t representation to enable storing the 
debug-info-carrying section (debug_types/debug_info) without decreasing the 
amount of debug info we can address (as would be the case with D61503 
).
- be a step towards supporting DWO files containing more than one unit 
(important for debug_types+dwo, but can also happen with regular dwo+lto). For 
this part to fully work we'd still need to add a way to lookup the 
SymbolFileDWO without going through GetCompileUnitAtIndex, but making sure 
things don't accidentally work because the SymbolFile ID is the same as compile 
unit offset is a step towards that.


https://reviews.llvm.org/D61783

Files:
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp


Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -23,7 +23,7 @@
DWARFUnit *dwarf_cu)
 : SymbolFileDWARF(objfile.get()), m_obj_file_sp(objfile),
   m_base_dwarf_cu(dwarf_cu) {
-  SetID(((lldb::user_id_t)dwarf_cu->GetOffset()) << 32);
+  SetID(((lldb::user_id_t)dwarf_cu->GetID()) << 32);
 }
 
 void SymbolFileDWARFDwo::LoadSectionData(lldb::SectionType sect_type,
@@ -158,6 +158,7 @@
 
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
-  lldbassert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset);
+  lldbassert(die_ref.cu_offset == m_base_dwarf_cu->GetOffset() ||
+ die_ref.cu_offset == DW_INVALID_OFFSET);
   return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset);
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1259,7 +1259,20 @@
 debug_map->GetOSOIndexFromUserID(uid));
 return {dwarf, {DW_INVALID_OFFSET, dw_offset_t(uid)}};
   }
-  return {this, {dw_offset_t(uid >> 32), dw_offset_t(uid)}};
+  uint32_t dwarf_id = uid >> 32;
+  dw_offset_t die_offset = uid;
+
+  if (die_offset == DW_INVALID_OFFSET)
+return {nullptr, DIERef()};
+
+  SymbolFileDWARF *dwarf = this;
+  if (DebugInfo()) {
+if (DWARFUnit *unit = DebugInfo()->GetCompileUnitAtIndex(dwarf_id)) {
+  if (unit->GetDwoSymbolFile())
+dwarf = unit->GetDwoSymbolFile();
+}
+  }
+  return {dwarf, {DW_INVALID_OFFSET, die_offset}};
 }
 
 DWARFDIE


Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -23,7 +23,7 @@
DWARFUnit *dwarf_cu)
 : SymbolFileDWARF(objfile.get()), m_obj_file_sp(objfile),
   m_base_dwarf_cu(dwarf_cu) {
-  SetID(((lldb::user_id_t)dwarf_cu->GetOffset()) << 32);
+  SetID(((lldb::user_id_t)dwarf_cu->GetID()) << 32);
 }
 
 void SymbolFileDWARFDwo::LoadSectionData(lldb::SectionType sect_type,
@@ -158,6 +158,7 @@
 
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
-  lldbassert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset);
+  lldbassert(die_ref.cu_offset == m_base_dwarf_cu->GetOffset() ||
+ die_ref.cu_offset == DW_INVALID_OFFSET);
   return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset);
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1259,7 +1259,20 @@
 debug_map->GetOSOIndexFromUserID(uid));
 return {dwarf, {DW_INVALID_OFFSET, dw_offset_t(uid)}};
   }
-  return {this, {dw_offset_t(uid >> 32), dw_offset_t(uid)}};
+  uint32_t dwarf_id = uid >> 32;
+  dw_offset_t die_offset = uid;
+
+  if (die_offset == DW_INVALID_OFFSET)
+return {nullptr, DIERef()};
+
+  SymbolFileDWARF *dwarf = this;
+  if (DebugInfo()) {
+if (DWARFUnit *unit = DebugInfo()->GetCompileUnitAtIndex(dwarf_id)) {
+  if (unit->GetDwoSymbolFile())
+dwarf = unit->GetDwoSymbolFile();
+}
+  }
+  return {dwarf, {DW_INVALID_OFFSET, die_offset}};
 }
 
 DWARFDIE
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r360423 - [lldb] [lit] Fix clobbers in x86_64 register test

2019-05-10 Thread Michal Gorny via lldb-commits
Author: mgorny
Date: Fri May 10 06:12:36 2019
New Revision: 360423

URL: http://llvm.org/viewvc/llvm-project?rev=360423&view=rev
Log:
[lldb] [lit] Fix clobbers in x86_64 register test

Modified:
lldb/trunk/lit/Register/Inputs/x86-64-write.cpp

Modified: lldb/trunk/lit/Register/Inputs/x86-64-write.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Register/Inputs/x86-64-write.cpp?rev=360423&r1=360422&r2=360423&view=diff
==
--- lldb/trunk/lit/Register/Inputs/x86-64-write.cpp (original)
+++ lldb/trunk/lit/Register/Inputs/x86-64-write.cpp Fri May 10 06:12:36 2019
@@ -55,8 +55,8 @@ int main() {
 "movaps  %%xmm15, 0x70(%1)\n\t"
 :
 : "a"(r64), "b"(xmm), "m"(xmm_fill)
-: "%mm0", "%mm1", "%mm2", "%mm3", "%mm4", "%mm5", "%mm6", "%mm7", "%xmm0",
-  "%xmm1", "%xmm2", "%xmm3", "%xmm4", "%xmm5", "%xmm6", "%xmm7"
+: "%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15", "%xmm8",
+  "%xmm9", "%xmm10", "%xmm11", "%xmm12", "%xmm13", "%xmm14", "%xmm15"
   );
 
   for (int i = 0; i < 8; ++i)


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


[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

2019-05-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: include/lldb/Symbol/FuncUnwinders.h:117
+  class LazyPlan {
+lldb::UnwindPlanSP m_plan_sp;
+

maybe use:

```
llvm::Optional m_plan_sp;
```

Then just check if it has no value, and if so compute and set it either to a 
valid shared pointer or an empty one?


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

https://reviews.llvm.org/D61779



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


[Lldb-commits] [PATCH] D61783: [DWARF] Use sequential integers for the IDs of the SymbolFileDWOs

2019-05-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Might want the person that did DWO support to chime in here if that wasn't you?


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

https://reviews.llvm.org/D61783



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


[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done.
labath added inline comments.



Comment at: include/lldb/Symbol/FuncUnwinders.h:117
+  class LazyPlan {
+lldb::UnwindPlanSP m_plan_sp;
+

clayborg wrote:
> maybe use:
> 
> ```
> llvm::Optional m_plan_sp;
> ```
> 
> Then just check if it has no value, and if so compute and set it either to a 
> valid shared pointer or an empty one?
That would work, but it would increase the size of the FuncUnwinders struct by 
about 80 bytes (8 bytes for each LazyPlan object). I can do that, but given 
that "uncomputed value" trick is internal to the class and does not leak out or 
affect the implementation, it seemed like a worthwhile optimization to me.


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

https://reviews.llvm.org/D61779



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


[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

2019-05-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: include/lldb/Symbol/FuncUnwinders.h:117
+  class LazyPlan {
+lldb::UnwindPlanSP m_plan_sp;
+

labath wrote:
> clayborg wrote:
> > maybe use:
> > 
> > ```
> > llvm::Optional m_plan_sp;
> > ```
> > 
> > Then just check if it has no value, and if so compute and set it either to 
> > a valid shared pointer or an empty one?
> That would work, but it would increase the size of the FuncUnwinders struct 
> by about 80 bytes (8 bytes for each LazyPlan object). I can do that, but 
> given that "uncomputed value" trick is internal to the class and does not 
> leak out or affect the implementation, it seemed like a worthwhile 
> optimization to me.
Actually isn't there a pointer union class that can steal bool bits from the 
aligned values and not increase the size?


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

https://reviews.llvm.org/D61779



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


[Lldb-commits] [PATCH] D61783: [DWARF] Use sequential integers for the IDs of the SymbolFileDWOs

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

It was @tberghammer. I'm cc-ing him to these patches, but I'm not sure what's 
his state these days...


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

https://reviews.llvm.org/D61783



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


[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

2019-05-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: include/lldb/Symbol/FuncUnwinders.h:117
+  class LazyPlan {
+lldb::UnwindPlanSP m_plan_sp;
+

clayborg wrote:
> labath wrote:
> > clayborg wrote:
> > > maybe use:
> > > 
> > > ```
> > > llvm::Optional m_plan_sp;
> > > ```
> > > 
> > > Then just check if it has no value, and if so compute and set it either 
> > > to a valid shared pointer or an empty one?
> > That would work, but it would increase the size of the FuncUnwinders struct 
> > by about 80 bytes (8 bytes for each LazyPlan object). I can do that, but 
> > given that "uncomputed value" trick is internal to the class and does not 
> > leak out or affect the implementation, it seemed like a worthwhile 
> > optimization to me.
> Actually isn't there a pointer union class that can steal bool bits from the 
> aligned values and not increase the size?
I believe the clang type system steals 3 bits from a Type * to encode "const" 
and a few other type qualifiers


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

https://reviews.llvm.org/D61779



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


[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

2019-05-10 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done.
labath added inline comments.



Comment at: include/lldb/Symbol/FuncUnwinders.h:117
+  class LazyPlan {
+lldb::UnwindPlanSP m_plan_sp;
+

clayborg wrote:
> clayborg wrote:
> > labath wrote:
> > > clayborg wrote:
> > > > maybe use:
> > > > 
> > > > ```
> > > > llvm::Optional m_plan_sp;
> > > > ```
> > > > 
> > > > Then just check if it has no value, and if so compute and set it either 
> > > > to a valid shared pointer or an empty one?
> > > That would work, but it would increase the size of the FuncUnwinders 
> > > struct by about 80 bytes (8 bytes for each LazyPlan object). I can do 
> > > that, but given that "uncomputed value" trick is internal to the class 
> > > and does not leak out or affect the implementation, it seemed like a 
> > > worthwhile optimization to me.
> > Actually isn't there a pointer union class that can steal bool bits from 
> > the aligned values and not increase the size?
> I believe the clang type system steals 3 bits from a Type * to encode "const" 
> and a few other type qualifiers
Yes, there is a `llvm::PointerIntPair`, but AFAICT that only works with a raw 
pointer, not a shared_ptr. I believe implementing a similar trick for 
shared_ptr would be possible, but it would definitely be tricky. Way more 
trickier than this..


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

https://reviews.llvm.org/D61779



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


[Lldb-commits] [lldb] r360432 - minidump: Don't eagerly resolve module paths read from the minidump

2019-05-10 Thread Pavel Labath via lldb-commits
Author: labath
Date: Fri May 10 08:05:26 2019
New Revision: 360432

URL: http://llvm.org/viewvc/llvm-project?rev=360432&view=rev
Log:
minidump: Don't eagerly resolve module paths read from the minidump

This can cause us to return paths to files on the local filesystem even
if we don't end up using that file (for instance because the file is not
a real module).

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/relative_module_name.yaml
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=360432&r1=360431&r2=360432&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Fri May 10 08:05:26 2019
@@ -34,7 +34,7 @@ class MiniDumpUUIDTestCase(TestBase):
 self.assertEqual(verify_uuid, uuid)
 
 def get_minidump_modules(self, yaml_file):
-minidump_path = self.getBuildArtifact(yaml_file + ".dmp")
+minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + 
".dmp")
 self.yaml2obj(yaml_file, minidump_path)
 self.target = self.dbg.CreateTarget(None)
 self.process = self.target.LoadCore(minidump_path)
@@ -166,3 +166,14 @@ class MiniDumpUUIDTestCase(TestBase):
 self.verify_module(modules[0],

"/invalid/path/on/current/system/libuuidmismatch.so", 
"7295E17C-6668-9E05-CBB5-DEE5003865D5")
+
+def test_relative_module_name(self):
+old_cwd = os.getcwd()
+self.addTearDownHook(lambda: os.chdir(old_cwd))
+os.chdir(self.getBuildDir())
+name = "file-with-a-name-unlikely-to-exist-in-the-current-directory.so"
+open(name, "a").close()
+modules = self.get_minidump_modules(
+self.getSourcePath("relative_module_name.yaml"))
+self.assertEqual(1, len(modules))
+self.verify_module(modules[0], name, None)

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/relative_module_name.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/relative_module_name.yaml?rev=360432&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/relative_module_name.yaml
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/relative_module_name.yaml
 Fri May 10 08:05:26 2019
@@ -0,0 +1,17 @@
+--- !minidump
+Streams: 
+  - Type:SystemInfo
+Processor Arch:  AMD64
+Platform ID: Linux
+CSD Version: '15E216'
+CPU: 
+  Vendor ID:   GenuineIntel
+  Version Info:0x
+  Feature Info:0x
+  - Type:ModuleList
+Modules: 
+  - Base of Image:   0x1000
+Size of Image:   0x1000
+Module Name: 
'file-with-a-name-unlikely-to-exist-in-the-current-directory.so'
+CodeView Record: ''
+...

Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=360432&r1=360431&r2=360432&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Fri May 10 
08:05:26 2019
@@ -368,7 +368,6 @@ void ProcessMinidump::ReadModuleList() {
 
 const auto uuid = m_minidump_parser->GetModuleUUID(module);
 auto file_spec = FileSpec(name, GetArchitecture().GetTriple());
-FileSystem::Instance().Resolve(file_spec);
 ModuleSpec module_spec(file_spec, uuid);
 module_spec.GetArchitecture() = GetArchitecture();
 Status error;


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


[Lldb-commits] [lldb] r360443 - Finish renaming CompileUnit -> Unit

2019-05-10 Thread Jan Kratochvil via lldb-commits
Author: jankratochvil
Date: Fri May 10 10:14:37 2019
New Revision: 360443

URL: http://llvm.org/viewvc/llvm-project?rev=360443&view=rev
Log:
Finish renaming CompileUnit -> Unit

D42892 changed a lot of code to use superclass DWARFUnit instead of its
subclass DWARFCompileUnit.

Finish this change more thoroughly for any *CompileUnit* -> *Unit* names.
Later patch will introduce DWARFTypeUnit which needs to be sometimes different
from DWARFCompileUnit and it would be confusing without this renaming.

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp?rev=360443&r1=360442&r2=360443&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp Fri May 10 
10:14:37 2019
@@ -74,8 +74,8 @@ void AppleDWARFIndex::GetGlobalVariables
 return;
 
   DWARFMappedHash::DIEInfoArray hash_data;
-  if (m_apple_names_up->AppendAllDIEsInRange(
-  cu.GetOffset(), cu.GetNextCompileUnitOffset(), hash_data))
+  if (m_apple_names_up->AppendAllDIEsInRange(cu.GetOffset(),
+ cu.GetNextUnitOffset(), 
hash_data))
 DWARFMappedHash::ExtractDIEArray(hash_data, offsets);
 }
 

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp?rev=360443&r1=360442&r2=360443&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp Fri May 10 
10:14:37 2019
@@ -52,8 +52,7 @@ llvm::Expected DWARFCompile
 cu_sp->m_addr_size = debug_info.GetU8(offset_ptr);
   }
 
-  bool length_OK =
-  debug_info.ValidOffset(cu_sp->GetNextCompileUnitOffset() - 1);
+  bool length_OK = debug_info.ValidOffset(cu_sp->GetNextUnitOffset() - 1);
   bool version_OK = SymbolFileDWARF::SupportedVersion(cu_sp->m_version);
   bool abbr_offset_OK =
   dwarf2Data->get_debug_abbrev_data().ValidOffset(abbr_offset);
@@ -85,7 +84,7 @@ void DWARFCompileUnit::Dump(Stream *s) c
 "abbr_offset = 0x%8.8x, addr_size = 0x%2.2x (next CU at "
 "{0x%8.8x})\n",
 m_offset, m_length, m_version, GetAbbrevOffset(), m_addr_size,
-GetNextCompileUnitOffset());
+GetNextUnitOffset());
 }
 
 uint32_t DWARFCompileUnit::GetHeaderByteSize() const {

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp?rev=360443&r1=360442&r2=360443&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp Fri May 10 10:14:37 
2019
@@ -154,7 +154,7 @@ DWARFDIE::LookupDeepestBlock(lldb::addr_
 if (cu->ContainsDIEOffset(block_die->GetOffset()))
   return DWARFDIE(cu, block_die);
 else
-  return DWARFDIE(dwarf->DebugInfo()->GetCompileUnit(
+  return DWARFDIE(dwarf->DebugInfo()->GetUnit(
   DIERef(cu->GetOffset(), block_die->GetOffset())),
   block_die);
   }

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp?rev=360443&r1=360442&r2=360443&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp Fri May 10 
10:14:37 2019
@@ -29,13 +29,12 @@ using namespace std;
 
 // Constructor
 DWARFDebugInfo::DWARFDebugInfo(lldb_pri

[Lldb-commits] [PATCH] D61501: 02/06: Finish renaming CompileUnit->Unit

2019-05-10 Thread Jan Kratochvil via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360443: Finish renaming CompileUnit -> Unit (authored by 
jankratochvil, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61501?vs=197983&id=199034#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61501

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -32,9 +32,9 @@
   Timer scoped_timer(func_cat, "%p", static_cast(&debug_info));
 
   std::vector units_to_index;
-  units_to_index.reserve(debug_info.GetNumCompileUnits());
-  for (size_t U = 0; U < debug_info.GetNumCompileUnits(); ++U) {
-DWARFUnit *unit = debug_info.GetCompileUnitAtIndex(U);
+  units_to_index.reserve(debug_info.GetNumUnits());
+  for (size_t U = 0; U < debug_info.GetNumUnits(); ++U) {
+DWARFUnit *unit = debug_info.GetUnitAtIndex(U);
 if (unit && m_units_to_avoid.count(unit->GetOffset()) == 0)
   units_to_index.push_back(unit);
   }
@@ -43,8 +43,8 @@
 
   std::vector sets(units_to_index.size());
 
-  // Keep memory down by clearing DIEs for any compile units if indexing
-  // caused us to load the compile unit's DIEs.
+  // Keep memory down by clearing DIEs for any units if indexing
+  // caused us to load the unit's DIEs.
   std::vector> clear_cu_dies(
   units_to_index.size());
   auto parser_fn = [&](size_t cu_idx) {
@@ -55,17 +55,17 @@
 clear_cu_dies[cu_idx] = units_to_index[cu_idx]->ExtractDIEsScoped();
   };
 
-  // Create a task runner that extracts dies for each DWARF compile unit in a
+  // Create a task runner that extracts dies for each DWARF unit in a
   // separate thread
-  // First figure out which compile units didn't have their DIEs already
+  // First figure out which units didn't have their DIEs already
   // parsed and remember this.  If no DIEs were parsed prior to this index
   // function call, we are going to want to clear the CU dies after we are
   // done indexing to make sure we don't pull in all DWARF dies, but we need
-  // to wait until all compile units have been indexed in case a DIE in one
-  // compile unit refers to another and the indexes accesses those DIEs.
+  // to wait until all units have been indexed in case a DIE in one
+  // unit refers to another and the indexes accesses those DIEs.
   TaskMapOverInt(0, units_to_index.size(), extract_fn);
 
-  // Now create a task runner that can index each DWARF compile unit in a
+  // Now create a task runner that can index each DWARF unit in a
   // separate thread so we can index quickly.
 
   TaskMapOverInt(0, units_to_index.size(), parser_fn);
@@ -95,7 +95,7 @@
 
   if (log) {
 m_module.LogMessage(
-log, "ManualDWARFIndex::IndexUnit for compile unit at .debug_info[0x%8.8x]",
+log, "ManualDWARFIndex::IndexUnit for unit at .debug_info[0x%8.8x]",
 unit.GetOffset());
   }
 
@@ -213,7 +213,7 @@
 //   if (block_data) {
 // uint32_t block_length = form_value.Unsigned();
 // if (block_length == 1 +
-// attributes.CompileUnitAtIndex(i)->GetAddressByteSize()) {
+// attributes.UnitAtIndex(i)->GetAddressByteSize()) {
 //   if (block_data[0] == DW_OP_addr)
 // add_die = true;
 // }
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -154,7 +154,7 @@
 if (cu->ContainsDIEOffset(block_die->GetOffset()))
   return DWARFDIE(cu, block_die);
 else
-  return DWARFDIE(dwarf->DebugInfo()->GetCompileUnit(
+  return DWARFDIE(dwarf->DebugInfo()->GetUnit(
   DIERef(cu->Get

[Lldb-commits] [PATCH] D61776: [Target] Generalize some behavior in Thread

2019-05-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

If you really are going to support many languages you need to figure out how to 
tell folks what really happened with more specificity.  For instance, you can 
use C++ to throw anything, so you could throw an ObjC object from a C++ 
exception throw.  So you need to distinguish between the language of the 
exception thrown (which is captured by the ValueObject you return) and the 
language runtime throwing the language.  So we need a way to query that.  Also, 
there's no formula reason why you couldn't have two languages throwing an 
exception at the same time (for instance if a C++ exception is unwinding the 
stack and the destructor of some ObjC object that is getting destroyed throws 
an NSException, etc...  So there needs to be some way to handle that.

This change isn't wrong but it gives a false impression of generality which 
makes it less well motivated.


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

https://reviews.llvm.org/D61776



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


[Lldb-commits] [PATCH] D61501: 02/06: Finish renaming CompileUnit->Unit

2019-05-10 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

Thanks for the review, I have pushed it ahead of the rest of the series as it 
was a bit annoying to keep it updating off-trunk. And then the renaming is a 
correct cleanup anyway.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61501



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


[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

I really don't want to pollute the lldb driver options with gdb equivalents (or 
really any duplicate spellings of already present functionality).  For the lldb 
command line, I want us to have the freedom to do the best job of making the 
lldb options consistent and easy to document, understand and use, and adding in 
random duplicate options from gdb will only make this harder.

If you have simple gdb startup commands then translating them into the lldb way 
should be straightforward, and the burden of learning how lldb works from the 
command line for these purposes is just not that great and if you are going to 
use lldb you should just buck up and browse "lldb --help" a bit...  If you have 
complex gdb command lines, you are going to have to rework them anyway to deal 
with other differences between lldb and gdb.  I don't think mixing the two 
would add enough value to be worth the uglification.  lldb is a different 
debugger from gdb and I think we should be fine with being that.

As to Eric's point, I don't think you would need to do any refactoring of lldb 
to create a gdb compatible shell on top of lldb.  The SB API's were sufficient 
to do the gdb-mi mode and should be sufficient for a gdb driver as well.  If 
you wanted to reuse the command parser code to make it run in lldb or gdb mode, 
that would take a bunch of work!  But I think you'd be crazy to do it that way. 
 The two command syntaxes are quite different, and I think you'd cause yourself 
much more trouble trying to jam the two together than you would gain from code 
sharing...  After all, the gdb command language is pretty much unstructured, 
and the command line is parsed ad hoc by the command that receives it, so 
implementing the parser part of gdb would be very little work, and you wouldn't 
be able to reuse the part of lldb that does structured commands since the gdb 
ones aren't.

As you got further along in the project, you'd probably have to add a few more 
customization points to lldb to implement all the gdb features.  For instance 
you'd probably want to be able to implement stop-hooks in something other than 
the lldb command line commands.  But that's all work that we would want to do 
in lldb anyway and just haven't gotten around to.  But you could probably get a 
pretty long way just on the current SB API's.

Finally, I agree with Raphael about IDE's.  There are SB API's to do all the 
things you can set up with command line options - and if there's anything 
missing there we can and should add it.  So if you want to make an IDE using 
lldb, you should be using the SB API's.  If you are trying to use the same IDE 
code to drive gdb & lldb you should be using the MI interface, which again can 
specify everything you need to run a session after you get started so you 
shouldn't need compatibility with driver options.  And if there's spare cycles 
and desire to support this sort of thing that effort would much better go into 
supporting the lldb-mi  which is currently languishing for lack of maintainers, 
than in trying to fool an IDE into thinking the lldb and gdb command lines are 
the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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


[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

In D61737#1498297 , @jingham wrote:

> If you have simple gdb startup commands then translating them into the lldb 
> way should be straightforward, and the burden of learning how lldb works from 
> the command line for these purposes is just not that great and if you are 
> going to use lldb you should just buck up and browse "lldb --help" a bit...  
> If you have complex gdb command lines, you are going to have to rework them 
> anyway to deal with other differences between lldb and gdb.  I don't think 
> mixing the two would add enough value to be worth the uglification.


I use GDB to debug LLDB like: `gdb -q -ex 'set break pend on' -ex 'set 
build-id-verbose 0' -ex 'set env 
ASAN_OPTIONS=alloc_dealloc_mismatch=0:detect_leaks=0' -ex 'set env 
LD_PRELOAD=/lib64/libasan.so.5' -ex "set env 
PYTHONPATH=$PWD/lib64/python2.7/site-packages:$PWD/lib64/python2.7/site-packages/lldb"
 -ex "set env LD_LIBRARY_PATH=$PWD/lib:$PWD/lib64/python2.7/site-packages/lldb" 
-iex 'set breakpoint pending on' -iex 'b __sanitizer::internal__exit' -ex 'b 
lldb_private::Module::ReportError' -ex 'b lldb_private::Host::SystemLog' -ex r 
-ex 'set scheduler-locking on' --args python 
../llvm-git/tools/lldb/test/dotest.py --executable $PWD/bin/lldb -C 
$PWD/bin/clang --log-success -t 
../llvm-git/tools/lldb/packages/Python/lldbsuite/test/  -f  
HelloWatchpointTestCase.test_hello_watchpoint_using_watchpoint_set_dwarf`

I would like to switch one day from GDB to LLDB but so far I soon face some 
issue why I cannot use  LLDB yet. It is usually some Fedora Linux port 
compatibility problem. It would be nice to just `s/gdb/lldb/` for the test 
without rewriting all the parameters only to find out there is still something 
remaining to port in LLDB to make the switch possible.

IIUC we should make a new GDB-like driver using `liblldb.so`+`SB API` for both 
GDB commandline parameters and GDB commands compatibility. For the GDB commands 
interpreter there could be a fallback to forward unknown commands to LLDB 
interpreter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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


[Lldb-commits] [PATCH] D61776: [Target] Generalize some behavior in Thread

2019-05-10 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In D61776#1498225 , @jingham wrote:

> If you really are going to support many languages you need to figure out how 
> to tell folks what really happened with more specificity.


I agree.

> For instance, you can use C++ to throw anything, so you could throw an ObjC 
> object from a C++ exception throw.  So you need to distinguish between the 
> language of the exception thrown (which is captured by the ValueObject you 
> return) and the language runtime throwing the language.  So we need a way to 
> query that.  Also, there's no formula reason why you couldn't have two 
> languages throwing an exception at the same time (for instance if a C++ 
> exception is unwinding the stack and the destructor of some ObjC object that 
> is getting destroyed throws an NSException, etc...  So there needs to be some 
> way to handle that.
> 
> This change isn't wrong but it gives a false impression of generality which 
> makes it less well motivated.

Better support for exception handling is definitely something we should work 
towards. This patch doesn't actually change behavior, but I understand your 
concern that generalizing will make it look like things are better supported 
than they actually are. My motivation behind this change is removing 
language-specific knowledge from Thread, which is a goal that I think is worth 
pursuing. I could preserve/modify the comments that were there noting that only 
ObjC works right now.


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

https://reviews.llvm.org/D61776



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


[Lldb-commits] [PATCH] D61776: [Target] Generalize some behavior in Thread

2019-05-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

With appropriate comments, this seems fine.


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

https://reviews.llvm.org/D61776



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


[Lldb-commits] [PATCH] D61776: [Target] Generalize some behavior in Thread

2019-05-10 Thread Alex Langford via Phabricator via lldb-commits
xiaobai updated this revision to Diff 199048.
xiaobai added a comment.

Add comments to give better context


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

https://reviews.llvm.org/D61776

Files:
  source/Target/Thread.cpp


Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -2209,25 +2209,31 @@
   if (auto e = recognized_frame->GetExceptionObject())
 return e;
 
-  // FIXME: For now, only ObjC exceptions are supported. This should really
-  // iterate over all language runtimes and ask them all to give us the current
-  // exception.
-  if (auto runtime = GetProcess()->GetObjCLanguageRuntime())
-if (auto e = runtime->GetExceptionObjectForThread(shared_from_this()))
-  return e;
+  // NOTE: Even though this behavior is generalized, only ObjC is actually
+  // supported at the moment.
+  for (unsigned lang = eLanguageTypeUnknown; lang < eNumLanguageTypes; lang++) 
{
+if (auto runtime = GetProcess()->GetLanguageRuntime(
+static_cast(lang)))
+  if (auto e = runtime->GetExceptionObjectForThread(shared_from_this()))
+return e;
+  }
 
   return ValueObjectSP();
 }
 
 ThreadSP Thread::GetCurrentExceptionBacktrace() {
   ValueObjectSP exception = GetCurrentException();
-  if (!exception) return ThreadSP();
+  if (!exception)
+return ThreadSP();
 
-  // FIXME: For now, only ObjC exceptions are supported. This should really
-  // iterate over all language runtimes and ask them all to give us the current
-  // exception.
-  auto runtime = GetProcess()->GetObjCLanguageRuntime();
-  if (!runtime) return ThreadSP();
+  // NOTE: Even though this behavior is generalized, only ObjC is actually
+  // supported at the moment.
+  for (unsigned lang = eLanguageTypeUnknown; lang < eNumLanguageTypes; lang++) 
{
+if (auto runtime = GetProcess()->GetLanguageRuntime(
+static_cast(lang)))
+  if (auto bt = runtime->GetBacktraceThreadFromException(exception))
+return bt;
+  }
 
-  return runtime->GetBacktraceThreadFromException(exception);
+  return ThreadSP();
 }


Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -2209,25 +2209,31 @@
   if (auto e = recognized_frame->GetExceptionObject())
 return e;
 
-  // FIXME: For now, only ObjC exceptions are supported. This should really
-  // iterate over all language runtimes and ask them all to give us the current
-  // exception.
-  if (auto runtime = GetProcess()->GetObjCLanguageRuntime())
-if (auto e = runtime->GetExceptionObjectForThread(shared_from_this()))
-  return e;
+  // NOTE: Even though this behavior is generalized, only ObjC is actually
+  // supported at the moment.
+  for (unsigned lang = eLanguageTypeUnknown; lang < eNumLanguageTypes; lang++) {
+if (auto runtime = GetProcess()->GetLanguageRuntime(
+static_cast(lang)))
+  if (auto e = runtime->GetExceptionObjectForThread(shared_from_this()))
+return e;
+  }
 
   return ValueObjectSP();
 }
 
 ThreadSP Thread::GetCurrentExceptionBacktrace() {
   ValueObjectSP exception = GetCurrentException();
-  if (!exception) return ThreadSP();
+  if (!exception)
+return ThreadSP();
 
-  // FIXME: For now, only ObjC exceptions are supported. This should really
-  // iterate over all language runtimes and ask them all to give us the current
-  // exception.
-  auto runtime = GetProcess()->GetObjCLanguageRuntime();
-  if (!runtime) return ThreadSP();
+  // NOTE: Even though this behavior is generalized, only ObjC is actually
+  // supported at the moment.
+  for (unsigned lang = eLanguageTypeUnknown; lang < eNumLanguageTypes; lang++) {
+if (auto runtime = GetProcess()->GetLanguageRuntime(
+static_cast(lang)))
+  if (auto bt = runtime->GetBacktraceThreadFromException(exception))
+return bt;
+  }
 
-  return runtime->GetBacktraceThreadFromException(exception);
+  return ThreadSP();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

2019-05-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D61737#1498329 , @jankratochvil 
wrote:

> In D61737#1498297 , @jingham wrote:
>
> > If you have simple gdb startup commands then translating them into the lldb 
> > way should be straightforward, and the burden of learning how lldb works 
> > from the command line for these purposes is just not that great and if you 
> > are going to use lldb you should just buck up and browse "lldb --help" a 
> > bit...  If you have complex gdb command lines, you are going to have to 
> > rework them anyway to deal with other differences between lldb and gdb.  I 
> > don't think mixing the two would add enough value to be worth the 
> > uglification.
>
>
> I use GDB to debug LLDB like: `gdb -q -ex 'set break pend on' -ex 'set 
> build-id-verbose 0' -ex 'set env 
> ASAN_OPTIONS=alloc_dealloc_mismatch=0:detect_leaks=0' -ex 'set env 
> LD_PRELOAD=/lib64/libasan.so.5' -ex "set env 
> PYTHONPATH=$PWD/lib64/python2.7/site-packages:$PWD/lib64/python2.7/site-packages/lldb"
>  -ex "set env 
> LD_LIBRARY_PATH=$PWD/lib:$PWD/lib64/python2.7/site-packages/lldb" -iex 'set 
> breakpoint pending on' -iex 'b __sanitizer::internal__exit' -ex 'b 
> lldb_private::Module::ReportError' -ex 'b lldb_private::Host::SystemLog' -ex 
> r -ex 'set scheduler-locking on' --args python 
> ../llvm-git/tools/lldb/test/dotest.py --executable $PWD/bin/lldb -C 
> $PWD/bin/clang --log-success -t 
> ../llvm-git/tools/lldb/packages/Python/lldbsuite/test/  -f  
> HelloWatchpointTestCase.test_hello_watchpoint_using_watchpoint_set_dwarf`


This is a good example.  Even if we added -ex  to mean the same thing as -o, 
this still wouldn't work because pretty much none of your set commands would 
work in lldb without modification.  So you still would have to port this 
command to lldb to get it to work, at which point having -ex mean the same as 
-o wouldn't really have helped you that much...

> I would like to switch one day from GDB to LLDB but so far I soon face some 
> issue why I cannot use  LLDB yet. It is usually some Fedora Linux port 
> compatibility problem. It would be nice to just `s/gdb/lldb/` for the test 
> without rewriting all the parameters only to find out there is still 
> something remaining to port in LLDB to make the switch possible.
> 
> IIUC we should make a new GDB-like driver using `liblldb.so`+`SB API` for 
> both GDB commandline parameters and GDB commands compatibility. For the GDB 
> commands interpreter there could be a fallback to forward unknown commands to 
> LLDB interpreter.

If someone wants to support this workflow, that would be the way to do it IMO.  
OTOH, I really don't want to end up again in the situation we are in with the 
lldb-mi where some folks did a bunch of work to get the basic functionality 
working, but it wasn't in the main flow of use of lldb, and the original 
authors lost interest and it isn't quite full-strength (e.g. doesn't work with 
the emacs MI adaptor)  and so we're left with a bunch of unsupported code that 
almost works and though it would be useful in certain cases it needs work and 
has flakey tests and nobody is really stepping up to fix things so it ends up 
being a burden.  That just makes lldb look bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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


[Lldb-commits] [PATCH] D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo()

2019-05-10 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik created this revision.
shafik added reviewers: friss, jingham, aam.

We track down a crash in FindLibCppStdFunctionCallableInfo() to a missing 
`nullptr` check for the `symbol` variable.


https://reviews.llvm.org/D61805

Files:
  source/Target/CPPLanguageRuntime.cpp


Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -278,7 +278,8 @@
   }
 
   // Case 4 or 5
-  if (!symbol->GetName().GetStringRef().startswith("vtable for")) {
+  if (symbol != nullptr &&
+  !symbol->GetName().GetStringRef().startswith("vtable for")) {
 optional_info.callable_case =
 LibCppStdFunctionCallableCase::FreeOrMemberFunction;
 optional_info.callable_address = function_address_resolved;


Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -278,7 +278,8 @@
   }
 
   // Case 4 or 5
-  if (!symbol->GetName().GetStringRef().startswith("vtable for")) {
+  if (symbol != nullptr &&
+  !symbol->GetName().GetStringRef().startswith("vtable for")) {
 optional_info.callable_case =
 LibCppStdFunctionCallableCase::FreeOrMemberFunction;
 optional_info.callable_address = function_address_resolved;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D61778: [Docs] Replace SVN revisions with lldb versions

2019-05-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM, Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61778



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


[Lldb-commits] [lldb] r360480 - Ted pointed out that some of test tests that are enabling packet

2019-05-10 Thread Jason Molenda via lldb-commits
Author: jmolenda
Date: Fri May 10 16:03:05 2019
New Revision: 360480

URL: http://llvm.org/viewvc/llvm-project?rev=360480&view=rev
Log:
Ted pointed out that some of test tests that are enabling packet
logging when the testsuite is run with trace mode enabled are leaving
the logging enabled after the tests have finished.  That state
isn't cleared in a --no-multiprocess testsuite run.


Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestTargetXMLArch.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py?rev=360480&r1=360479&r2=360480&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
 Fri May 10 16:03:05 2019
@@ -109,9 +109,7 @@ class TestArmRegisterDefinition(GDBRemot
 
 self.server.responder = MyResponder()
 if self.TraceOn():
-interp = self.dbg.GetCommandInterpreter()
-result = lldb.SBCommandReturnObject()
-interp.HandleCommand("log enable gdb-remote packets", result)
+self.runCmd("log enable gdb-remote packets")
 self.dbg.SetDefaultArchitecture("armv7em")
 target = self.dbg.CreateTargetWithFileAndArch(None, None)
 
@@ -128,3 +126,6 @@ class TestArmRegisterDefinition(GDBRemot
 
 pc_valobj = 
process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("pc")
 self.assertEqual(pc_valobj.GetValueAsUnsigned(), 0x0800d22e)
+
+if self.TraceOn():
+self.runCmd("log disable gdb-remote packets")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py?rev=360480&r1=360479&r2=360480&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py
 Fri May 10 16:03:05 2019
@@ -40,9 +40,7 @@ class TestNoWatchpointSupportInfo(GDBRem
 
 self.server.responder = MyResponder()
 if self.TraceOn():
-interp = self.dbg.GetCommandInterpreter()
-result = lldb.SBCommandReturnObject()
-interp.HandleCommand("log enable gdb-remote packets", result)
+self.runCmd("log enable gdb-remote packets")
 self.dbg.SetDefaultArchitecture("x86_64")
 target = self.dbg.CreateTargetWithFileAndArch(None, None)
 
@@ -62,3 +60,7 @@ class TestNoWatchpointSupportInfo(GDBRem
 err.GetDescription(strm)
 print("watchpoint failed: %s" % strm.GetData())
 self.assertTrue(wp.IsValid())
+
+if self.TraceOn():
+self.runCmd("log disable gdb-remote packets")
+

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py?rev=360480&r1=360479&r2=360480&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
 Fri May 10 16:03:05 2019
@@ -44,3 +44,6 @@ class TestStopPCs(GDBRemoteTestBase):
 self.assertEqual(th1.GetThreadID(), 0x2ff0d)
 self.assertEqual(th0.GetFrameAtIndex(0).GetPC(), 0x10001bc00)
 self.assertEqual(th1.GetFrameAtIndex(0).GetPC(), 0x10002bc00)
+
+if self.TraceOn():
+  self.runCmd("log disable gdb-remote packets")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestTargetXMLArch.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestTargetXMLArch.py?rev=360480&r1=360479&r2=360480&view=diff
==
--- 

[Lldb-commits] [lldb] r360482 - Change the disabling of packet logging to be in TearDownHook lambdas.

2019-05-10 Thread Jason Molenda via lldb-commits
Author: jmolenda
Date: Fri May 10 16:22:15 2019
New Revision: 360482

URL: http://llvm.org/viewvc/llvm-project?rev=360482&view=rev
Log:
Change the disabling of packet logging to be in TearDownHook lambdas.


Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestTargetXMLArch.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py?rev=360482&r1=360481&r2=360482&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
 Fri May 10 16:22:15 2019
@@ -110,6 +110,9 @@ class TestArmRegisterDefinition(GDBRemot
 self.server.responder = MyResponder()
 if self.TraceOn():
 self.runCmd("log enable gdb-remote packets")
+self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+
 self.dbg.SetDefaultArchitecture("armv7em")
 target = self.dbg.CreateTargetWithFileAndArch(None, None)
 
@@ -126,6 +129,3 @@ class TestArmRegisterDefinition(GDBRemot
 
 pc_valobj = 
process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("pc")
 self.assertEqual(pc_valobj.GetValueAsUnsigned(), 0x0800d22e)
-
-if self.TraceOn():
-self.runCmd("log disable gdb-remote packets")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py?rev=360482&r1=360481&r2=360482&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py
 Fri May 10 16:22:15 2019
@@ -41,6 +41,8 @@ class TestNoWatchpointSupportInfo(GDBRem
 self.server.responder = MyResponder()
 if self.TraceOn():
 self.runCmd("log enable gdb-remote packets")
+self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
 self.dbg.SetDefaultArchitecture("x86_64")
 target = self.dbg.CreateTargetWithFileAndArch(None, None)
 
@@ -61,6 +63,3 @@ class TestNoWatchpointSupportInfo(GDBRem
 print("watchpoint failed: %s" % strm.GetData())
 self.assertTrue(wp.IsValid())
 
-if self.TraceOn():
-self.runCmd("log disable gdb-remote packets")
-

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py?rev=360482&r1=360481&r2=360482&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
 Fri May 10 16:22:15 2019
@@ -35,6 +35,8 @@ class TestStopPCs(GDBRemoteTestBase):
 target = self.dbg.CreateTarget('')
 if self.TraceOn():
   self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
 process = self.connect(target)
 
 self.assertEqual(process.GetNumThreads(), 2)
@@ -44,6 +46,3 @@ class TestStopPCs(GDBRemoteTestBase):
 self.assertEqual(th1.GetThreadID(), 0x2ff0d)
 self.assertEqual(th0.GetFrameAtIndex(0).GetPC(), 0x10001bc00)
 self.assertEqual(th1.GetFrameAtIndex(0).GetPC(), 0x10002bc00)
-
-if self.TraceOn():
-  self.runCmd("log disable gdb-remote packets")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestTargetXMLArch.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestTargetXMLArch.py?rev=360482&r1=360481&r2=360482&view=diff
===

[Lldb-commits] [PATCH] D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo()

2019-05-10 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment.

Do you have a testcase triggering this?


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

https://reviews.llvm.org/D61805



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


[Lldb-commits] [PATCH] D61759: Switch to FindSymbolsMatchingRegExAndType() from FindFunctions() in FindLibCppStdFunctionCallableInfo()

2019-05-10 Thread Frederic Riss via Phabricator via lldb-commits
friss accepted this revision.
friss added a comment.
This revision is now accepted and ready to land.

This seems obviously better, go for it.


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

https://reviews.llvm.org/D61759



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


[Lldb-commits] [PATCH] D61746: [Breakpoint] Make breakpoint language agnostic

2019-05-10 Thread Alex Langford via Phabricator via lldb-commits
xiaobai updated this revision to Diff 199113.
xiaobai added a comment.

- Fix minor bug
- Return vector by value instead of passing in one by parameter.


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

https://reviews.llvm.org/D61746

Files:
  include/lldb/Target/Language.h
  source/Breakpoint/BreakpointResolverName.cpp
  source/Breakpoint/CMakeLists.txt
  source/Plugins/Language/ObjC/ObjCLanguage.cpp
  source/Plugins/Language/ObjC/ObjCLanguage.h

Index: source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- source/Plugins/Language/ObjC/ObjCLanguage.h
+++ source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -73,18 +73,6 @@
 
 ConstString GetSelector();
 
-// Get all possible names for a method. Examples:
-// If name is "+[NSString(my_additions) myStringWithCString:]"
-//  names[0] => "+[NSString(my_additions) myStringWithCString:]"
-//  names[1] => "+[NSString myStringWithCString:]"
-// If name is specified without the leading '+' or '-' like
-// "[NSString(my_additions) myStringWithCString:]"
-//  names[0] => "+[NSString(my_additions) myStringWithCString:]"
-//  names[1] => "-[NSString(my_additions) myStringWithCString:]"
-//  names[2] => "+[NSString myStringWithCString:]"
-//  names[3] => "-[NSString myStringWithCString:]"
-size_t GetFullNames(std::vector &names, bool append);
-
   protected:
 ConstString
 m_full; // Full name:   "+[NSString(my_additions) myStringWithCString:]"
@@ -105,6 +93,18 @@
 return lldb::eLanguageTypeObjC;
   }
 
+  // Get all possible names for a method. Examples:
+  // If method_name is "+[NSString(my_additions) myStringWithCString:]"
+  //   variant_names[0] => "+[NSString myStringWithCString:]"
+  // If name is specified without the leading '+' or '-' like
+  // "[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[0] => "+[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[1] => "-[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[2] => "+[NSString myStringWithCString:]"
+  //  variant_names[3] => "-[NSString myStringWithCString:]"
+  std::vector
+  GetMethodNameVariants(ConstString method_name) const override;
+
   lldb::TypeCategoryImplSP GetFormatters() override;
 
   std::vector
Index: source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -220,43 +220,46 @@
   return ConstString();
 }
 
-size_t ObjCLanguage::MethodName::GetFullNames(std::vector &names,
-  bool append) {
-  if (!append)
-names.clear();
-  if (IsValid(false)) {
+std::vector
+ObjCLanguage::GetMethodNameVariants(ConstString method_name) const {
+  std::vector variant_names;
+  ObjCLanguage::MethodName objc_method(method_name.GetCString(), false);
+  if (!objc_method.IsValid(false)) {
+return variant_names;
+  }
+
+  const bool is_class_method =
+  objc_method.GetType() == MethodName::eTypeClassMethod;
+  const bool is_instance_method =
+  objc_method.GetType() == MethodName::eTypeInstanceMethod;
+  ConstString name_sans_category =
+  objc_method.GetFullNameWithoutCategory(/*empty_if_no_category*/ true);
+
+  if (is_class_method || is_instance_method) {
+if (name_sans_category)
+  variant_names.emplace_back(name_sans_category);
+  } else {
 StreamString strm;
-const bool is_class_method = m_type == eTypeClassMethod;
-const bool is_instance_method = m_type == eTypeInstanceMethod;
-ConstString category = GetCategory();
-if (is_class_method || is_instance_method) {
-  names.push_back(m_full);
-  if (category) {
-strm.Printf("%c[%s %s]", is_class_method ? '+' : '-',
-GetClassName().GetCString(), GetSelector().GetCString());
-names.emplace_back(strm.GetString());
-  }
-} else {
-  ConstString class_name = GetClassName();
-  ConstString selector = GetSelector();
-  strm.Printf("+[%s %s]", class_name.GetCString(), selector.GetCString());
-  names.emplace_back(strm.GetString());
-  strm.Clear();
-  strm.Printf("-[%s %s]", class_name.GetCString(), selector.GetCString());
-  names.emplace_back(strm.GetString());
+
+strm.Printf("+%s", objc_method.GetFullName().GetCString());
+variant_names.emplace_back(strm.GetString());
+strm.Clear();
+
+strm.Printf("-%s", objc_method.GetFullName().GetCString());
+variant_names.emplace_back(strm.GetString());
+strm.Clear();
+
+if (name_sans_category) {
+  strm.Printf("+%s", name_sans_category.GetCString());
+  variant_names.emplace_back(strm.GetString());
   strm.Clear();
-  if (category) {
-strm.Printf("+[%s(%s) %s]", class_name.GetCString(),
-category.GetCString(), selector.GetCString());
-names

[Lldb-commits] [PATCH] D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo()

2019-05-10 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

What's the scenario that's causing this? Adding a nullptr check is an obviously 
save thing to do, but it would be excellent if we could add a comment 
explaining why the symbol could be nullptr.
I also do believe that the logic for checking whether the symbol is nullptr can 
be hoisted to the beginning of the function, see comment inline.




Comment at: source/Target/CPPLanguageRuntime.cpp:217-218
 
 if (symbol != NULL &&
 symbol->GetName().GetStringRef().contains("__invoke")) {
 

This should probably be `nullptr`, anyway, my general comment is that this 
check is scattered all around the function and could be centralized in a single 
place.



Comment at: source/Target/CPPLanguageRuntime.cpp:262-276
   if (first_template_parameter.contains("$_") ||
   (symbol != nullptr &&
symbol->GetName().GetStringRef().contains("__invoke"))) {
 // Case 1 and 2
 optional_info.callable_case = LibCppStdFunctionCallableCase::Lambda;
   } else {
 // Case 3

Here in the `if` branch you check whether the symbol is nullptr or not, but 
later you dereference it unconditionally. Are you always guaranteed that you're 
not dereferencing `nullptr` ?


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

https://reviews.llvm.org/D61805



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


[Lldb-commits] [lldb] r360509 - [Breakpoint] Make breakpoint language agnostic

2019-05-10 Thread Alex Langford via lldb-commits
Author: xiaobai
Date: Fri May 10 20:32:25 2019
New Revision: 360509

URL: http://llvm.org/viewvc/llvm-project?rev=360509&view=rev
Log:
[Breakpoint] Make breakpoint language agnostic

Summary:
Breakpoint shouldn't need to depend on any specific details from a
programming language. Currently the only language-specific detail it takes
advantage of are the different qualified names an objective-c method name might
have when adding a name lookup. This is reasonably generalizable.

The current method name I introduced is "GetVariantMethodNames", which I'm not
particularly tied to. If you have a better suggestion, please do let me know.

Reviewers: JDevlieghere, jingham, clayborg

Subscribers: mgorny, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Target/Language.h
lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
lldb/trunk/source/Breakpoint/CMakeLists.txt
lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h

Modified: lldb/trunk/include/lldb/Target/Language.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Language.h?rev=360509&r1=360508&r2=360509&view=diff
==
--- lldb/trunk/include/lldb/Target/Language.h (original)
+++ lldb/trunk/include/lldb/Target/Language.h Fri May 10 20:32:25 2019
@@ -190,6 +190,14 @@ public:
 
   virtual const char *GetLanguageSpecificTypeLookupHelp();
 
+  // If a language can have more than one possible name for a method, this
+  // function can be used to enumerate them. This is useful when doing name
+  // lookups.
+  virtual std::vector
+  GetMethodNameVariants(ConstString method_name) const {
+return std::vector();
+  };
+
   // if an individual data formatter can apply to several types and cross a
   // language boundary it makes sense for individual languages to want to
   // customize the printing of values of that type by appending proper

Modified: lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp?rev=360509&r1=360508&r2=360509&view=diff
==
--- lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp Fri May 10 20:32:25 
2019
@@ -8,7 +8,6 @@
 
 #include "lldb/Breakpoint/BreakpointResolverName.h"
 
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Architecture.h"
 #include "lldb/Core/Module.h"
@@ -17,6 +16,7 @@
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
@@ -218,20 +218,27 @@ StructuredData::ObjectSP BreakpointResol
 
 void BreakpointResolverName::AddNameLookup(ConstString name,
FunctionNameType name_type_mask) {
-  ObjCLanguage::MethodName objc_method(name.GetCString(), false);
-  if (objc_method.IsValid(false)) {
-std::vector objc_names;
-objc_method.GetFullNames(objc_names, true);
-for (ConstString objc_name : objc_names) {
-  Module::LookupInfo lookup;
-  lookup.SetName(name);
-  lookup.SetLookupName(objc_name);
-  lookup.SetNameTypeMask(eFunctionNameTypeFull);
-  m_lookups.push_back(lookup);
+
+  Module::LookupInfo lookup(name, name_type_mask, m_language);
+  m_lookups.emplace_back(lookup);
+
+  auto add_variant_funcs = [&](Language *lang) {
+for (ConstString variant_name : lang->GetMethodNameVariants(name)) {
+  Module::LookupInfo variant_lookup(name, name_type_mask,
+lang->GetLanguageType());
+  variant_lookup.SetLookupName(variant_name);
+  m_lookups.emplace_back(variant_lookup);
 }
+return true;
+  };
+
+  if (Language *lang = Language::FindPlugin(m_language)) {
+add_variant_funcs(lang);
   } else {
-Module::LookupInfo lookup(name, name_type_mask, m_language);
-m_lookups.push_back(lookup);
+// Most likely m_language is eLanguageTypeUnknown. We check each language 
for
+// possible variants or more qualified names and create lookups for those 
as
+// well.
+Language::ForEach(add_variant_funcs);
   }
 }
 

Modified: lldb/trunk/source/Breakpoint/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/CMakeLists.txt?rev=360509&r1=360508&r2=360509&view=diff
==
--- lldb/trunk/source/Breakpoint/CMakeLists.txt (original)
+++ lldb/trunk/source/Breakpoint/CMakeLists.txt Fri May 10 20:32:25 2019
@@ -30,7 +30,6 @@ add_lldb_library(lldbBreakpoint
 lldbSymbol
 lldbTarget
 lldbUtili

[Lldb-commits] [PATCH] D61746: [Breakpoint] Make breakpoint language agnostic

2019-05-10 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360509: [Breakpoint] Make breakpoint language agnostic 
(authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61746

Files:
  lldb/trunk/include/lldb/Target/Language.h
  lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
  lldb/trunk/source/Breakpoint/CMakeLists.txt
  lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h

Index: lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -73,18 +73,6 @@
 
 ConstString GetSelector();
 
-// Get all possible names for a method. Examples:
-// If name is "+[NSString(my_additions) myStringWithCString:]"
-//  names[0] => "+[NSString(my_additions) myStringWithCString:]"
-//  names[1] => "+[NSString myStringWithCString:]"
-// If name is specified without the leading '+' or '-' like
-// "[NSString(my_additions) myStringWithCString:]"
-//  names[0] => "+[NSString(my_additions) myStringWithCString:]"
-//  names[1] => "-[NSString(my_additions) myStringWithCString:]"
-//  names[2] => "+[NSString myStringWithCString:]"
-//  names[3] => "-[NSString myStringWithCString:]"
-size_t GetFullNames(std::vector &names, bool append);
-
   protected:
 ConstString
 m_full; // Full name:   "+[NSString(my_additions) myStringWithCString:]"
@@ -105,6 +93,18 @@
 return lldb::eLanguageTypeObjC;
   }
 
+  // Get all possible names for a method. Examples:
+  // If method_name is "+[NSString(my_additions) myStringWithCString:]"
+  //   variant_names[0] => "+[NSString myStringWithCString:]"
+  // If name is specified without the leading '+' or '-' like
+  // "[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[0] => "+[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[1] => "-[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[2] => "+[NSString myStringWithCString:]"
+  //  variant_names[3] => "-[NSString myStringWithCString:]"
+  std::vector
+  GetMethodNameVariants(ConstString method_name) const override;
+
   lldb::TypeCategoryImplSP GetFormatters() override;
 
   std::vector
Index: lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -220,43 +220,46 @@
   return ConstString();
 }
 
-size_t ObjCLanguage::MethodName::GetFullNames(std::vector &names,
-  bool append) {
-  if (!append)
-names.clear();
-  if (IsValid(false)) {
+std::vector
+ObjCLanguage::GetMethodNameVariants(ConstString method_name) const {
+  std::vector variant_names;
+  ObjCLanguage::MethodName objc_method(method_name.GetCString(), false);
+  if (!objc_method.IsValid(false)) {
+return variant_names;
+  }
+
+  const bool is_class_method =
+  objc_method.GetType() == MethodName::eTypeClassMethod;
+  const bool is_instance_method =
+  objc_method.GetType() == MethodName::eTypeInstanceMethod;
+  ConstString name_sans_category =
+  objc_method.GetFullNameWithoutCategory(/*empty_if_no_category*/ true);
+
+  if (is_class_method || is_instance_method) {
+if (name_sans_category)
+  variant_names.emplace_back(name_sans_category);
+  } else {
 StreamString strm;
-const bool is_class_method = m_type == eTypeClassMethod;
-const bool is_instance_method = m_type == eTypeInstanceMethod;
-ConstString category = GetCategory();
-if (is_class_method || is_instance_method) {
-  names.push_back(m_full);
-  if (category) {
-strm.Printf("%c[%s %s]", is_class_method ? '+' : '-',
-GetClassName().GetCString(), GetSelector().GetCString());
-names.emplace_back(strm.GetString());
-  }
-} else {
-  ConstString class_name = GetClassName();
-  ConstString selector = GetSelector();
-  strm.Printf("+[%s %s]", class_name.GetCString(), selector.GetCString());
-  names.emplace_back(strm.GetString());
-  strm.Clear();
-  strm.Printf("-[%s %s]", class_name.GetCString(), selector.GetCString());
-  names.emplace_back(strm.GetString());
+
+strm.Printf("+%s", objc_method.GetFullName().GetCString());
+variant_names.emplace_back(strm.GetString());
+strm.Clear();
+
+strm.Printf("-%s", objc_method.GetFullName().GetCString());
+variant_names.emplace_back(strm.GetString());
+strm.Clear();
+
+if (name_sans_category) {
+  strm.Printf("+%s", name_sans_category.GetCStri